Merge lp:~akretion-team/project-service/project-service-base-sale-project into lp:~project-core-editors/project-service/trunk

Proposed by Sébastien BEAU - http://www.akretion.com
Status: Merged
Merged at revision: 32
Proposed branch: lp:~akretion-team/project-service/project-service-base-sale-project
Merge into: lp:~project-core-editors/project-service/trunk
Diff against target: 272 lines (+241/-0)
6 files modified
sale_project_base/__init__.py (+26/-0)
sale_project_base/__openerp__.py (+43/-0)
sale_project_base/i18n/sale_project_base.po (+38/-0)
sale_project_base/project.py (+37/-0)
sale_project_base/sale.py (+69/-0)
sale_project_base/sale_view.xml (+28/-0)
To merge this branch: bzr merge lp:~akretion-team/project-service/project-service-base-sale-project
Reviewer Review Type Date Requested Status
Daniel Reis lgtm, no test Approve
Pedro Manuel Baeza code review Approve
Review via email: mp+211960@code.launchpad.net

Description of the change

Hi,
We are cleanning and improving our OpenERP, so we plan to contribute everything here !
Let's start with some basic module
Here it's just a base module in order to link correctly a project and a sale order and also to create a project from a sale order.
No big thing, it's an base module that can be usefull in many case

Thanks for your review

To post a comment you must log in.
Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Hi, Sébastien, some remarks from a quick scan:

- Why do you need to declare a new field 'true_project_id' instead of using existing 'project_id'?
- You can put auto_install to True so that if sale and project are installed, this module is automatically installed.
- A similar module that glues sale and stock is called sale_stock. Maybe you can call yours sale_project.

Thanks for the contribution.

Regards.

review: Needs Information (code review)
Revision history for this message
Sébastien BEAU - http://www.akretion.com (sebastien.beau) wrote :

Hi Pedro

In fact this module is not a glue it more a base module for our modules (they will be contributed). But we think other project can need it so it why I push it here right now.

This module just add basic but usefull link between order and project and give the possibility to create a project from the sale order. I am not sure that it's a good idea to install it by default, I think it's better to install it only if it's needed. This is why I call it base_sale_project because it should be use as dependency of other module.

Regarding the field "project_id". In our case we need to have a real link between the sale order and the project. As "project_id" is already used for adding a link with... analytic.account :'(. We need to have a field that point on the "project.project" we don't found a better name than "true_project_id". The best will to clean addons code... and replace "project_id" by "analytic_account_id"

Thanks for your feedback

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

Yeah, you're right about project_id relation with analytic account instead of the project.

About names and so on, it's a matter of taste, so I'm not going to object.

I have checked code and seems OK, so I give my approval.

What is what we expect next?

Regards.

review: Approve (code review)
Revision history for this message
Daniel Reis (dreis-pt) wrote :

Thanks for the contribution!

Generating Projects from Sales Orders is a possible workflow.
But generating Tasks from Sales orders is also a valid workflow.
So I don't think it's a good idea to have it "autoinstall"ed.

I agree with Pedro on the two other issues:
- "base_*" modules are usually core functionality related; how about "sale_project_base"?
- I also find that "true_project_id" is not very intuitive. How about something like "related_project_id" or "sale_project_id"?

Nitpicks:
L66: s/posibility/possibility
L182: methods intended to be called by a button tend to be called "action_*". I suggest renaming this to "action_create_project()".
L188: I believe the context==None control can be safely removed here

Another suggestion (could be a later improvement):
- It would be nice for the create button to return an action, so that it opens the created Project after the button is clicked; like the Issue's "Create Task" button (standard in v6, ported to v7 in a project-service module).

review: Needs Information
38. By Sébastien BEAU - http://www.akretion.com

[REF] change module name, rename method linked to the button, remove useless context is None

Revision history for this message
Sébastien BEAU - http://www.akretion.com (sebastien.beau) wrote :

@Daniel
I did the change, the name sale_project_base seem perfect for me.

For the field name, we should found something that contrast with the native "project_id" to avoid confusion. Developper that read the code should be alarmed by the name of the field.

Maybe "real_project_id"? What do you think?

note : related_project_id is great also. Pedro which one do you prefers?

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

I think related_project_id can be intuitive enough and not a shocking name.

Daniel suggestions are good for me too.

Regards.

Revision history for this message
Raphaël Valyi - http://www.akretion.com (rvalyi) wrote :

IMHO real_project_id and true_project id looks the same.
For us keeping true_project_id would make migration easier...
I also think "true_project_id" carries enough "subversive meta information" to make it clear that if a foreign key points to an analytic account and not to a project, may be it was not a good idea to call it "project_id".

Thoughts?

Revision history for this message
Daniel Reis (dreis-pt) wrote :

If you're already using the module, changing a field name is rather painful, and it probably isn't worth the effort. So, no objections there.

It would also be nice to add the i18n/.po file, but I won't pick on that.

review: Approve (lgtm, no test)
Revision history for this message
Daniel Reis (dreis-pt) wrote :

PS: Raphael, having a "project_id" field pointing at a Analytic Account is the kind of thing that bring emotion to OpenERP ;-)

39. By Sébastien BEAU - http://www.akretion.com

[REF] rename true_project_id into related_project_id

40. By Sébastien BEAU - http://www.akretion.com

[IMP] add translation file

Revision history for this message
Sébastien BEAU - http://www.akretion.com (sebastien.beau) wrote :

I talk with Raph.
Even if we have to migrate our work, the later we will do the change the more painful it will be so let's rename it!
I also add the .po file

Thanks for your feedback

Just one question what is the best name for the onchange method?
- onchange_my_field
- on_my_field_change
-....

Revision history for this message
Pedro Manuel Baeza (pedro.baeza) wrote :

I use to call it onchange_fieldname: compact and indicating the field name.

Regards.

41. By Sébastien BEAU - http://www.akretion.com

[REF] rename onchange and pass the context

Revision history for this message
Sébastien BEAU - http://www.akretion.com (sebastien.beau) wrote :

Yes is seem to be the best option, let's update my code

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== added directory 'sale_project_base'
=== added file 'sale_project_base/__init__.py'
--- sale_project_base/__init__.py 1970-01-01 00:00:00 +0000
+++ sale_project_base/__init__.py 2014-03-20 19:06:11 +0000
@@ -0,0 +1,26 @@
1# -*- coding: utf-8 -*-
2###############################################################################
3#
4# Module for OpenERP
5# Copyright (C) 2014 Akretion (http://www.akretion.com).
6# Copyright (C) 2010-2013 Akretion LDTA (<http://www.akretion.com>)
7# @author Sébastien BEAU <sebastien.beau@akretion.com>
8# @author Benoît GUILLOT <benoit.guillot@akretion.com>
9#
10# This program is free software: you can redistribute it and/or modify
11# it under the terms of the GNU Affero General Public License as
12# published by the Free Software Foundation, either version 3 of the
13# License, or (at your option) any later version.
14#
15# This program is distributed in the hope that it will be useful,
16# but WITHOUT ANY WARRANTY; without even the implied warranty of
17# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
18# GNU Affero General Public License for more details.
19#
20# You should have received a copy of the GNU Affero General Public License
21# along with this program. If not, see <http://www.gnu.org/licenses/>.
22#
23###############################################################################
24
25from . import sale
26from . import project
027
=== added file 'sale_project_base/__openerp__.py'
--- sale_project_base/__openerp__.py 1970-01-01 00:00:00 +0000
+++ sale_project_base/__openerp__.py 2014-03-20 19:06:11 +0000
@@ -0,0 +1,43 @@
1# -*- coding: utf-8 -*-
2###############################################################################
3#
4# Module for OpenERP
5# Copyright (C) 2010-2013 Akretion LDTA (<http://www.akretion.com>).
6# Copyright (C) 2013 Akretion (http://www.akretion.com).
7# @author Benoît GUILLOT <benoit.guillot@akretion.com>
8#
9# This program is free software: you can redistribute it and/or modify
10# it under the terms of the GNU Affero General Public License as
11# published by the Free Software Foundation, either version 3 of the
12# License, or (at your option) any later version.
13#
14# This program is distributed in the hope that it will be useful,
15# but WITHOUT ANY WARRANTY; without even the implied warranty of
16# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
17# GNU Affero General Public License for more details.
18#
19# You should have received a copy of the GNU Affero General Public License
20# along with this program. If not, see <http://www.gnu.org/licenses/>.
21#
22###############################################################################
23
24
25{
26 'name': 'Sale Project Base',
27 'version': '7.0',
28 'category': 'Generic Modules/Others',
29 'license': 'AGPL-3',
30 'description': """This module is a base module that give the possibility to
31 create a project from a quotation""",
32 'author': 'Akretion',
33 'website': 'http://www.akretion.com/',
34 'depends': [
35 'project',
36 'sale',
37 ],
38 'demo': [],
39 'data': [
40 'sale_view.xml',
41 ],
42 'installable': True,
43}
044
=== added directory 'sale_project_base/i18n'
=== added file 'sale_project_base/i18n/sale_project_base.po'
--- sale_project_base/i18n/sale_project_base.po 1970-01-01 00:00:00 +0000
+++ sale_project_base/i18n/sale_project_base.po 2014-03-20 19:06:11 +0000
@@ -0,0 +1,38 @@
1# Translation of OpenERP Server.
2# This file contains the translation of the following modules:
3# * sale_project_base
4#
5msgid ""
6msgstr ""
7"Project-Id-Version: OpenERP Server 7.0\n"
8"Report-Msgid-Bugs-To: \n"
9"POT-Creation-Date: 2014-03-20 17:47+0000\n"
10"PO-Revision-Date: 2014-03-20 17:47+0000\n"
11"Last-Translator: <>\n"
12"Language-Team: \n"
13"MIME-Version: 1.0\n"
14"Content-Type: text/plain; charset=UTF-8\n"
15"Content-Transfer-Encoding: \n"
16"Plural-Forms: \n"
17
18#. module: sale_project_base
19#: model:ir.model,name:sale_project_base.model_project_project
20#: field:sale.order,related_project_id:0
21msgid "Project"
22msgstr ""
23
24#. module: sale_project_base
25#: model:ir.model,name:sale_project_base.model_sale_order
26msgid "Sales Order"
27msgstr ""
28
29#. module: sale_project_base
30#: view:sale.order:0
31msgid "Create Project"
32msgstr ""
33
34#. module: sale_project_base
35#: field:project.project,order_ids:0
36msgid "Orders"
37msgstr ""
38
039
=== added file 'sale_project_base/project.py'
--- sale_project_base/project.py 1970-01-01 00:00:00 +0000
+++ sale_project_base/project.py 2014-03-20 19:06:11 +0000
@@ -0,0 +1,37 @@
1# -*- coding: utf-8 -*-
2###############################################################################
3#
4# Module for OpenERP
5# Copyright (C) 2014 Akretion (http://www.akretion.com).
6# Copyright (C) 2010-2013 Akretion LDTA (<http://www.akretion.com>)
7# @author Sébastien BEAU <sebastien.beau@akretion.com>
8# @author Benoît GUILLOT <benoit.guillot@akretion.com>
9#
10# This program is free software: you can redistribute it and/or modify
11# it under the terms of the GNU Affero General Public License as
12# published by the Free Software Foundation, either version 3 of the
13# License, or (at your option) any later version.
14#
15# This program is distributed in the hope that it will be useful,
16# but WITHOUT ANY WARRANTY; without even the implied warranty of
17# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
18# GNU Affero General Public License for more details.
19#
20# You should have received a copy of the GNU Affero General Public License
21# along with this program. If not, see <http://www.gnu.org/licenses/>.
22#
23###############################################################################
24
25
26from openerp.osv import fields, orm
27
28
29class project_project(orm.Model):
30 _inherit = "project.project"
31
32 _columns = {
33 'order_ids': fields.one2many(
34 'sale.order',
35 'related_project_id',
36 'Orders'),
37 }
038
=== added file 'sale_project_base/sale.py'
--- sale_project_base/sale.py 1970-01-01 00:00:00 +0000
+++ sale_project_base/sale.py 2014-03-20 19:06:11 +0000
@@ -0,0 +1,69 @@
1# -*- coding: utf-8 -*-
2###############################################################################
3#
4# Module for OpenERP
5# Copyright (C) 2014 Akretion (http://www.akretion.com).
6# Copyright (C) 2010-2013 Akretion LDTA (<http://www.akretion.com>)
7# @author Sébastien BEAU <sebastien.beau@akretion.com>
8# @author Benoît GUILLOT <benoit.guillot@akretion.com>
9#
10# This program is free software: you can redistribute it and/or modify
11# it under the terms of the GNU Affero General Public License as
12# published by the Free Software Foundation, either version 3 of the
13# License, or (at your option) any later version.
14#
15# This program is distributed in the hope that it will be useful,
16# but WITHOUT ANY WARRANTY; without even the implied warranty of
17# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
18# GNU Affero General Public License for more details.
19#
20# You should have received a copy of the GNU Affero General Public License
21# along with this program. If not, see <http://www.gnu.org/licenses/>.
22#
23###############################################################################
24
25from openerp.osv import fields, orm
26from datetime import date
27
28
29class sale_order(orm.Model):
30 _inherit = "sale.order"
31
32 _columns = {
33 'related_project_id': fields.many2one(
34 'project.project',
35 'Project',
36 readonly=True,
37 states={'draft': [('readonly', False)]}
38 ),
39 }
40
41 def onchange_related_project_id(self, cr, uid, ids, related_project_id, context=None):
42 project_obj = self.pool['project.project']
43 if related_project_id:
44 project = project_obj.browse(cr, uid, related_project_id, context=context)
45 return {'value': {'project_id': project.analytic_account_id.id}}
46 return {}
47
48 def _prepare_project_vals(self, cr, uid, order, context=None):
49 name = u" %s - %s - %s" % (
50 order.partner_id.name,
51 date.today().year,
52 order.name)
53 return {
54 'user_id': order.user_id.id,
55 'name': name,
56 'partner_id': order.partner_id.id,
57 }
58
59 def action_create_project(self, cr, uid, ids, context=None):
60 project_obj = self.pool['project.project']
61 for order in self.browse(cr, uid, ids, context=context):
62 vals = self._prepare_project_vals(cr, uid, order, context)
63 project_id = project_obj.create(cr, uid, vals, context=context)
64 project = project_obj.browse(cr, uid, project_id, context=context)
65 order.write({
66 'related_project_id': project_id,
67 'project_id': project.analytic_account_id.id
68 })
69 return True
070
=== added file 'sale_project_base/sale_view.xml'
--- sale_project_base/sale_view.xml 1970-01-01 00:00:00 +0000
+++ sale_project_base/sale_view.xml 2014-03-20 19:06:11 +0000
@@ -0,0 +1,28 @@
1<?xml version="1.0" encoding="utf-8"?>
2<openerp>
3 <data>
4
5 <record id="view_order_form" model="ir.ui.view">
6 <field name="name">view_project_quotation_order_form</field>
7 <field name="model">sale.order</field>
8 <field name="inherit_id" ref="sale.view_order_form" />
9 <field name="arch" type="xml">
10 <field name="project_id" position="replace">
11 <field name="project_id" invisible="1" />
12 <group col="4" colspan="2">
13 <field name="related_project_id"
14 on_change="onchange_related_project_id(related_project_id, context)"
15 domain="[('partner_id','=',partner_id)]"
16 attrs="{'readonly':[('partner_id','=', False)]}"/>
17 <group attrs="{'invisible':[('related_project_id', '!=', False)]}">
18 <button name="action_create_project" states="draft"
19 string="Create Project"
20 type="object" icon="gtk-execute" />
21 </group>
22 </group>
23 </field>
24 </field>
25 </record>
26
27 </data>
28</openerp>

Subscribers

People subscribed via source and target branches