Merge lp:~vauxoo/openobject-addons/7.0-project_task_test_yaml_multi-company into lp:openobject-addons/7.0

Status: Work in progress
Proposed branch: lp:~vauxoo/openobject-addons/7.0-project_task_test_yaml_multi-company
Merge into: lp:openobject-addons/7.0
Diff against target: 181 lines (+150/-1)
4 files modified
project/__openerp__.py (+5/-1)
project/demo/task_multicompany_demo.yml (+87/-0)
project/test/task_mail_multicompany_test.yml (+23/-0)
project/test/task_multicompany_user_test.yml (+35/-0)
To merge this branch: bzr merge lp:~vauxoo/openobject-addons/7.0-project_task_test_yaml_multi-company
Reviewer Review Type Date Requested Status
Nhomar - Vauxoo (community) Needs Information
Jorge Angel Naranjo Rogel - http://www.vauxoo.com (community) Needs Resubmitting
Olivier Dony (Odoo) Pending
OpenERP R&D Team Pending
Moisés López - http://www.vauxoo.com Pending
Review via email: mp+183530@code.launchpad.net

Description of the change

Added test yaml for assign tasks with users multi-company

To post a comment you must log in.
9409. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[FIX] Fixed messages

Revision history for this message
Jorge Angel Naranjo Rogel - http://www.vauxoo.com (jorge-nr) wrote :

Fixed messages of test yaml task_multicompany

review: Needs Resubmitting
Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

This branch is a Work in Progress, when it is solved in our OPW this test will say everything is Ok!-

9410. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Added test yaml in demo

9411. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Send mail message from userA to userAB

9412. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Setting Receive Messages by Email to None

9413. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[FIX][project] Fixed test yaml task_multicompany

9414. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP] Validate res_users

9415. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Separated test 1 and 2

9416. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Improvemente in message of assert

9417. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Changed of user to create task ''Task assigned to UserAB by UserA with company A''

Revision history for this message
Moisés López - http://www.vauxoo.com (moylop260) wrote :

We change the test to see the demo data created in RunBot.
We also changed the focus of the test to show two important cases
1. Error mail.message view
2. Operation error to assign tasks to users who regularly change company

POINT # 1:
You can see in http://runbot.openerp.com/vauxoo.html
Instance: 7.0-project_task_test_yaml_multi-company
Connect with
user: userab
password: userab
You can see the error in mail.message view
https://docs.google.com/file/d/0BwPeHBuUYqNsODZ5NEFzOU94czg/edit?usp=sharing
You can check log here:
2013-09-10 20:58:52,202 27345 INFO 7-0-project-task-test-yaml-multi-company-19725-all openerp.modules.loading: module project: loading test/task_mail_multicompany_test.yml
2013-09-10 20:58:52,208 27345 TEST 7-0-project-task-test-yaml-multi-company-19725-all openerp.tools.yaml_import: Test 1, Assigned task to UserA with Multi-Company user
2013-09-10 20:58:52,208 27345 TEST 7-0-project-task-test-yaml-multi-company-19725-all openerp.tools.yaml_import: Read task of UserAB with UserA
2013-09-10 20:58:52,211 27345 TEST 7-0-project-task-test-yaml-multi-company-19725-all openerp.tools.yaml_import: Read task of UserAB with UserA
2013-09-10 20:58:52,214 27345 TEST 7-0-project-task-test-yaml-multi-company-19725-all openerp.tools.yaml_import: Read mail of UserAB with CompanyB
2013-09-10 20:58:52,238 27345 ERROR 7-0-project-task-test-yaml-multi-company-19725-all openerp.tools.yaml_import: AssertionError in Python code : Wrong. UserAB with company B can read my message, but I can not see the attached task.

POINT # 2:
2013-09-10 20:58:52,238 27345 INFO 7-0-project-task-test-yaml-multi-company-19725-all openerp.modules.loading: module project: loading test/task_multicompany_user_test.yml
2013-09-10 20:58:52,242 27345 TEST 7-0-project-task-test-yaml-multi-company-19725-all openerp.tools.yaml_import: Test 2, Assigned task to User Multi-Company with User A
2013-09-10 20:58:52,243 27345 TEST 7-0-project-task-test-yaml-multi-company-19725-all openerp.tools.yaml_import: Assigned companyA in UserAB
2013-09-10 20:58:52,261 27345 TEST 7-0-project-task-test-yaml-multi-company-19725-all openerp.tools.yaml_import: Create task assigned to UserAB with company A create by UserA
2013-09-10 20:58:52,369 27345 TEST 7-0-project-task-test-yaml-multi-company-19725-all openerp.tools.yaml_import: Changed companyA to companyB in UserAB
2013-09-10 20:58:52,387 27345 TEST 7-0-project-task-test-yaml-multi-company-19725-all openerp.tools.yaml_import: Task assigned to UserAB with company B create by UserA
2013-09-10 20:58:52,399 27345 ERROR 7-0-project-task-test-yaml-multi-company-19725-all openerp.tools.yaml_import: AssertionError in Python code : Wrong. No userAB found. The user can not attached to this task

I UserA create a first task to UserAB when I want create a second task to UserAB this not exist.

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Thanks for the update!

The errors that are highlighted by your tests are now quite clear. They are however the normal consequences of the way multi-company is designed in OpenERP at the moment. My comment (#3) on bug 1210263 explains why this is the expected behavior and gives some suggested workarounds for cases where this approach is not suitable.

This is however a case-by-case customization, as we cannot globally decide that the default multi-company rules should be less restrictive, or allow users who work in multiple companies to see records from all their companies all the time (very high chance of inconsistent data mix).

I hope you understand our position...

This may open up the debate about how multi-company works in OpenERP, but I'm not sure how we could keep it simple while still allowing this amount of flexibility. If UserAB is allowed to see documents from CompanyA and CompanyB all the time it will be a big mess, and there does not seem to be any other generic way to pass your tests?

Revision history for this message
Moisés López - http://www.vauxoo.com (moylop260) wrote :

Hello Oliver,

We understand you position with comment:
"Allow users who work in multiple companies to see records from all their companies all the time (very high chance of inconsistent data mix)."

We are agree with this.
We don't want mix project.task
We don't want mix purchase.order
We don't want mix sale.order
We knows inconsistent data mix in this models.

We have a functional problem that the res.users hiding or showing when it changes company_id, but other users lose control for this too.
Assign task, see responsible of purchases, add followers... (all models with res.users relation).
Then we need set a standard and special solution.

IMHO the unique model with posibility of data mixing should be res.users with company_ids ACL
No project.task, no purchase.order data mix.
Only res.users. We may assign records to users regardless if he changed his company and not generate data mix in other models.

E.G.
user1_company_ids = [1,2,3,4]
user2_company_ids = [4,5,6]
user1_read_users2_ok = any( [True for user1_company_id in user1_company_ids if user1_company_id in user2_company_ids] )
if user1_read_users2_ok:
   print "User1 read users2 because has a common company in company_ids"
else:
   print "User1 don't read users2"

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Hello,

As I told Nhomar, I understand the idea but I still think it is more of a customization than a valid change for the core.

First, there's the issue of res.partner in addition to res.users: in many places a partner is used instead of a user (e.g. for followers, email recipients, etc.). Due to the recent bugfix for bug 1073087, partners will follow the user's company at all times. So it's not just users, and the rule for partners would become more complicated, as they don't have "company_ids" directly.

Next, if you could write a rule that allows users to see other users if they have at least one company in common, it seems it will easily allow mixing incorrect values. Imagine userABC can see userCDE because they have company C in common, this would mean they could mix up data and assign userCDE to a document from CompanyA, which userCDE would never be able to see. I agree that it may not make any business sense to do that, but it would be possible.

It seems to me that any complete multi-company deployment includes a lot of review/fine-tuning of the default rules to adapt them to the company's business rules, and this issue should simply be considered as part of it, don't you think?

That said, multi-company is clearly an area in OpenERP that deserves improvements and research in usability, but I think that requires a more profound rework of the system (like adding assistants to help setup multi-company rules but also data like locations, shops, etc.)

Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

Hello Oliver, thanks for the feedback.

Lets explain better trying to show our point.

Messages are blocking business objects! IT IS a problem __always__ no matter why.

The unique solution we found is overwrite the ACL including the concept of company_ids, you say it is incorrect by definition, we understand, but, The workaround?

WE ARE AGREED :: It is a case-by-case analysis, ok, agreed, can we work in the "Case of Users."

Have multiples users per user brake any multi-company deploy because imagine i have 100 Users per comapny, with 40 sharing companies (real case) and 6 companies (for fiscal and logistic reasons it is possible and a real case) then I will have 40x6 + specific users = +300 users, it means __only__ for enterprise reasons is a crazy scenario, didn't say for administration PoV.

We build some modules that help, like clear permissions, build our own groups and so on, but in this point we are a little confused, why? IMHO because the multi-company ACL for users __must__ be in the core.

I make you by twitter an example let me be more deep here.

What you are thinking is a "Customization" is like Linus Torvalds says "Ok say users they must be care about security in /root folder and /usr/bin" and installing linux by default when i create a limited users the machine does't run in "Some Specific Places" because the *bin* folder in " blocked " by default and -It is an expected behavior.

Good It is my complain ;-) sorry... Now the solution.

We propose you in this MP, exchange with us what do you think is a Secure Way to deal with this specific scenario "May be proposing specifically what ACL we should use" and we can test it, and may be between you and us.

1.- Propose an MP for Trunk.
2.- Propose an MP for 7.0

The Solution that "Conceptually" we should achieve is:

If I need to show some "Fields" from the core in multi-company environments the model ItSelf must manage and deal with this Fields in an special way.

In this point messages query user's names it is the problem, can we Show the Name? only, build an ACL for it, and make generic?, Show the name and block the rest of fields like password and so on?

Can you enlighten us here, because I can't see the solution quickly as you say.

I understand very well ACL if you propose the "domain" to deal with it Great, but even without use python to overwrite the behavior your statement is telling us something what is Impossible i think.

For sure I don't have problems if this job needs some consultancy hours, but I can't see the solution now.

Thanks for your time dude really thanks.

review: Needs Information

Unmerged revisions

9417. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Changed of user to create task ''Task assigned to UserAB by UserA with company A''

9416. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Improvemente in message of assert

9415. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Separated test 1 and 2

9414. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP] Validate res_users

9413. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[FIX][project] Fixed test yaml task_multicompany

9412. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Setting Receive Messages by Email to None

9411. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Send mail message from userA to userAB

9410. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[IMP][project] Added test yaml in demo

9409. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[FIX] Fixed messages

9408. By Jorge Angel Naranjo Rogel - http://www.vauxoo.com

[FIX] Added ' in fields of type string

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'project/__openerp__.py'
2--- project/__openerp__.py 2013-01-31 18:41:41 +0000
3+++ project/__openerp__.py 2013-09-10 20:53:32 +0000
4@@ -77,11 +77,15 @@
5 'board_project_view.xml',
6 'res_config_view.xml',
7 ],
8- 'demo': ['project_demo.xml'],
9+ 'demo': ['project_demo.xml',
10+ 'demo/task_multicompany_demo.yml',
11+ ],
12 'test': [
13 'test/project_demo.yml',
14 'test/project_process.yml',
15 'test/task_process.yml',
16+ 'test/task_mail_multicompany_test.yml',
17+ 'test/task_multicompany_user_test.yml',
18 ],
19 'installable': True,
20 'auto_install': False,
21
22=== added directory 'project/demo'
23=== added file 'project/demo/task_multicompany_demo.yml'
24--- project/demo/task_multicompany_demo.yml 1970-01-01 00:00:00 +0000
25+++ project/demo/task_multicompany_demo.yml 2013-09-10 20:53:32 +0000
26@@ -0,0 +1,87 @@
27+-
28+ Test 1, Assigned task to UserA with Multi-Company user
29+-
30+ Create company 'CompanyTestA'
31+-
32+ !record {model: res.company, id: res_company_companyTestA, view: base.view_company_form}:
33+ name: 'CompanyTestA'
34+-
35+ Create company 'CompanyTestB'
36+-
37+ !record {model: res.company, id: res_company_companyTestB, view: base.view_company_form}:
38+ name: 'CompanyTestB'
39+-
40+ Create user 'UserAB'
41+-
42+ !record {model: res.users, id: res_users_AB, view: base.view_users_form}:
43+ company_id: base.main_company
44+ name: 'UserAB'
45+ login: 'userab'
46+ password: 'userab'
47+ notification_email_send: 'none'
48+ email: 'userab@example.com'
49+-
50+ Create user 'UserA'
51+-
52+ !record {model: res.users, id: res_users_A, view: base.view_users_form}:
53+ company_id: base.main_company
54+ name: 'UserA'
55+ login: 'usera'
56+ password: 'usera'
57+ notification_email_send: 'none'
58+ email: 'usera@example.com'
59+-
60+ Added companies to UserAB
61+-
62+ !python {model: res.users}: |
63+ data = self.read(cr,uid, [ref('base.user_root')], ['groups_id','company_ids'], context=context)
64+ groups_ids=data[0].get('groups_id')
65+ groups_ids.append(ref('base.group_multi_company'))
66+ data = self.read(cr,uid, [ref('res_users_AB')], ['groups_id','company_ids'], context=context)
67+ company_ids=data[0].get('company_ids')
68+ company_ids.append(ref('res_company_companyTestA'))
69+ company_ids.append(ref('res_company_companyTestB'))
70+ self.write(cr,uid, [ref('res_users_AB')], {'groups_id': [(6, 0, groups_ids)] ,'company_ids': [(6, 0, company_ids)],'company_id': ref('res_company_companyTestA')}, context=context)
71+-
72+ Added companies to UserA
73+-
74+ !python {model: res.users}: |
75+ data = self.read(cr,uid, [ref('base.user_root')], ['groups_id','company_ids'], context=context)
76+ groups_ids=data[0].get('groups_id')
77+ groups_ids.append(ref('base.group_multi_company'))
78+ data = self.read(cr,uid, [ref('res_users_A')], ['groups_id','company_ids'], context=context)
79+ company_ids=data[0].get('company_ids')
80+ company_ids.append(ref('res_company_companyTestA'))
81+ self.write(cr,uid, [ref('res_users_A')], {'groups_id': [(6, 0, groups_ids)] ,'company_ids': [(6, 0, company_ids)],'company_id': ref('res_company_companyTestA')}, context=context)
82+-
83+ Create task assigned to UserAB
84+-
85+ !python {model: project.task}: |
86+ vals = {'name':'Task assigned to UserAB by UserA with company A','user_id': ref('res_users_AB')}
87+ self.create(cr, ref('res_users_A'), vals, context=context)
88+-
89+ Changed companyA to companyB in UserAB
90+-
91+ !python {model: res.users}: |
92+ self.write(cr,uid, [ref('res_users_AB')], {'company_id': ref('res_company_companyTestB')}, context=context)
93+-
94+ Send mail message from userA to userAB
95+-
96+ !python {model: mail.message}: |
97+ task_id = self.pool.get('project.task').search(cr, ref('res_users_A'), [('name','like','Task assigned to UserAB by UserA with company A')])[0]
98+ partner_id = self.pool.get('res.users').browse(cr, ref('res_users_A'), [ref('res_users_A')])[0].partner_id.id
99+ mail_parent_id = self.search(cr, ref('res_users_A'), [('res_id', '=', task_id )])[0]
100+ vals = {'subject': 'Do you remember this task?',
101+ 'model': 'project.task',
102+ 'res_id': task_id ,
103+ 'body': """Hello. I would like to know that happened with this task. """,
104+ 'type': 'email',
105+ 'subtype_id': ref('mail.mt_comment'),
106+ 'parent_id': mail_parent_id,
107+ 'email_from': 'usera@example.com',
108+ 'author_id': partner_id,
109+ }
110+ self.create(cr, ref('res_users_A'), vals, context=context)
111+
112+
113+
114
115=== added file 'project/test/task_mail_multicompany_test.yml'
116--- project/test/task_mail_multicompany_test.yml 1970-01-01 00:00:00 +0000
117+++ project/test/task_mail_multicompany_test.yml 2013-09-10 20:53:32 +0000
118@@ -0,0 +1,23 @@
119+-
120+ Test 1, Assigned task to UserA with Multi-Company user
121+-
122+ Read task of UserAB with UserA
123+-
124+ !python {model: project.task}: |
125+ task_id = self.search(cr, ref('res_users_A'), [('name','like','Task assigned to UserAB by UserA with company A')])
126+ assert len(self.read(cr, ref('res_users_A'), task_id , ['name','user_id'])) > 0, 'Wrong. UserA with company A can not read this task'
127+-
128+ Read task of UserAB with UserA
129+-
130+ !python {model: project.task}: |
131+ task_id = self.search(cr, ref('res_users_A'), [('name','like','Task assigned to UserAB by UserA with company A')])
132+ assert len(self.read(cr, ref('res_users_A'), task_id , ['name','user_id'])) > 0, 'Wrong. UserA with company A can not read this task'
133+-
134+ Read mail of UserAB with CompanyB
135+-
136+ !python {model: mail.message}: |
137+ mail_id = self.search(cr, ref('res_users_AB'), [('subject','like','Do you remember this task?')])
138+ assert len(mail_id) > 0, 'Wrong. UserAB with company B can not read this mail'
139+ mail_read = self.read(cr, ref('res_users_AB'), mail_id , ['subject','body','res_id'])
140+ task_ids = self.pool.get('project.task').search(cr, ref('res_users_AB'), [('id','=',mail_read[0].get('res_id'))])
141+ assert len(task_ids) > 0, 'Wrong. UserAB with company B can read my message, but I can not see the attached task.'
142
143=== added file 'project/test/task_multicompany_user_test.yml'
144--- project/test/task_multicompany_user_test.yml 1970-01-01 00:00:00 +0000
145+++ project/test/task_multicompany_user_test.yml 2013-09-10 20:53:32 +0000
146@@ -0,0 +1,35 @@
147+-
148+ Test 2, Assigned task to User Multi-Company with User A
149+-
150+ Assigned companyA in UserAB
151+-
152+ !python {model: res.users}: |
153+ self.write(cr,uid, [ref('res_users_AB')], {'company_id': ref('res_company_companyTestA')}, context=context)
154+-
155+ Create task assigned to UserAB with company A create by UserA
156+-
157+ !python {model: project.task}: |
158+ res_user_id = self.pool.get('res.users').search(cr, ref('res_users_A') , [('id','=',ref('res_users_AB'))])
159+ assert len(res_user_id) > 0, 'Wrong. No userAB found '
160+ vals = {
161+ 'name':'Task assigned to UserAB with company A create by UserA',
162+ 'user_id': res_user_id[0],
163+ }
164+ self.create(cr, ref('res_users_A'), vals, context=context)
165+-
166+ Changed companyA to companyB in UserAB
167+-
168+ !python {model: res.users}: |
169+ self.write(cr,uid, [ref('res_users_AB')], {'company_id': ref('res_company_companyTestB')}, context=context)
170+-
171+ Task assigned to UserAB with company B create by UserA
172+-
173+ !python {model: project.task}: |
174+ res_user_id = self.pool.get('res.users').search(cr, ref('res_users_A') , [('id','=',ref('res_users_AB'))])
175+ assert len(res_user_id) > 0, 'Wrong. No userAB found. The user can not attached to this task'
176+ vals = {
177+ 'name':'Task assigned to UserAB with company B create by UserA',
178+ 'user_id': res_user_id[0] ,
179+ }
180+ self.create(cr, ref('res_users_A'), vals, context=context)
181+