Merge lp:~akretion-team/project-service/project-service-base-sale-project into lp:~project-core-editors/project-service/trunk
- project-service-base-sale-project
- Merge into trunk
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 |
Related bugs: |
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 |
Commit message
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
Pedro Manuel Baeza (pedro.baeza) wrote : | # |
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_
Thanks for your feedback
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.
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_
- I also find that "true_project_id" is not very intuitive. How about something like "related_
Nitpicks:
L66: s/posibility/
L182: methods intended to be called by a button tend to be called "action_*". I suggest renaming this to "action_
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).
- 38. By Sébastien BEAU - http://www.akretion.com
-
[REF] change module name, rename method linked to the button, remove useless context is None
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?
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.
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?
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.
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
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
-....
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
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
1 | === added directory 'sale_project_base' | |||
2 | === added file 'sale_project_base/__init__.py' | |||
3 | --- sale_project_base/__init__.py 1970-01-01 00:00:00 +0000 | |||
4 | +++ sale_project_base/__init__.py 2014-03-20 19:06:11 +0000 | |||
5 | @@ -0,0 +1,26 @@ | |||
6 | 1 | # -*- coding: utf-8 -*- | ||
7 | 2 | ############################################################################### | ||
8 | 3 | # | ||
9 | 4 | # Module for OpenERP | ||
10 | 5 | # Copyright (C) 2014 Akretion (http://www.akretion.com). | ||
11 | 6 | # Copyright (C) 2010-2013 Akretion LDTA (<http://www.akretion.com>) | ||
12 | 7 | # @author Sébastien BEAU <sebastien.beau@akretion.com> | ||
13 | 8 | # @author Benoît GUILLOT <benoit.guillot@akretion.com> | ||
14 | 9 | # | ||
15 | 10 | # This program is free software: you can redistribute it and/or modify | ||
16 | 11 | # it under the terms of the GNU Affero General Public License as | ||
17 | 12 | # published by the Free Software Foundation, either version 3 of the | ||
18 | 13 | # License, or (at your option) any later version. | ||
19 | 14 | # | ||
20 | 15 | # This program is distributed in the hope that it will be useful, | ||
21 | 16 | # but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
22 | 17 | # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
23 | 18 | # GNU Affero General Public License for more details. | ||
24 | 19 | # | ||
25 | 20 | # You should have received a copy of the GNU Affero General Public License | ||
26 | 21 | # along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
27 | 22 | # | ||
28 | 23 | ############################################################################### | ||
29 | 24 | |||
30 | 25 | from . import sale | ||
31 | 26 | from . import project | ||
32 | 0 | 27 | ||
33 | === added file 'sale_project_base/__openerp__.py' | |||
34 | --- sale_project_base/__openerp__.py 1970-01-01 00:00:00 +0000 | |||
35 | +++ sale_project_base/__openerp__.py 2014-03-20 19:06:11 +0000 | |||
36 | @@ -0,0 +1,43 @@ | |||
37 | 1 | # -*- coding: utf-8 -*- | ||
38 | 2 | ############################################################################### | ||
39 | 3 | # | ||
40 | 4 | # Module for OpenERP | ||
41 | 5 | # Copyright (C) 2010-2013 Akretion LDTA (<http://www.akretion.com>). | ||
42 | 6 | # Copyright (C) 2013 Akretion (http://www.akretion.com). | ||
43 | 7 | # @author Benoît GUILLOT <benoit.guillot@akretion.com> | ||
44 | 8 | # | ||
45 | 9 | # This program is free software: you can redistribute it and/or modify | ||
46 | 10 | # it under the terms of the GNU Affero General Public License as | ||
47 | 11 | # published by the Free Software Foundation, either version 3 of the | ||
48 | 12 | # License, or (at your option) any later version. | ||
49 | 13 | # | ||
50 | 14 | # This program is distributed in the hope that it will be useful, | ||
51 | 15 | # but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
52 | 16 | # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
53 | 17 | # GNU Affero General Public License for more details. | ||
54 | 18 | # | ||
55 | 19 | # You should have received a copy of the GNU Affero General Public License | ||
56 | 20 | # along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
57 | 21 | # | ||
58 | 22 | ############################################################################### | ||
59 | 23 | |||
60 | 24 | |||
61 | 25 | { | ||
62 | 26 | 'name': 'Sale Project Base', | ||
63 | 27 | 'version': '7.0', | ||
64 | 28 | 'category': 'Generic Modules/Others', | ||
65 | 29 | 'license': 'AGPL-3', | ||
66 | 30 | 'description': """This module is a base module that give the possibility to | ||
67 | 31 | create a project from a quotation""", | ||
68 | 32 | 'author': 'Akretion', | ||
69 | 33 | 'website': 'http://www.akretion.com/', | ||
70 | 34 | 'depends': [ | ||
71 | 35 | 'project', | ||
72 | 36 | 'sale', | ||
73 | 37 | ], | ||
74 | 38 | 'demo': [], | ||
75 | 39 | 'data': [ | ||
76 | 40 | 'sale_view.xml', | ||
77 | 41 | ], | ||
78 | 42 | 'installable': True, | ||
79 | 43 | } | ||
80 | 0 | 44 | ||
81 | === added directory 'sale_project_base/i18n' | |||
82 | === added file 'sale_project_base/i18n/sale_project_base.po' | |||
83 | --- sale_project_base/i18n/sale_project_base.po 1970-01-01 00:00:00 +0000 | |||
84 | +++ sale_project_base/i18n/sale_project_base.po 2014-03-20 19:06:11 +0000 | |||
85 | @@ -0,0 +1,38 @@ | |||
86 | 1 | # Translation of OpenERP Server. | ||
87 | 2 | # This file contains the translation of the following modules: | ||
88 | 3 | # * sale_project_base | ||
89 | 4 | # | ||
90 | 5 | msgid "" | ||
91 | 6 | msgstr "" | ||
92 | 7 | "Project-Id-Version: OpenERP Server 7.0\n" | ||
93 | 8 | "Report-Msgid-Bugs-To: \n" | ||
94 | 9 | "POT-Creation-Date: 2014-03-20 17:47+0000\n" | ||
95 | 10 | "PO-Revision-Date: 2014-03-20 17:47+0000\n" | ||
96 | 11 | "Last-Translator: <>\n" | ||
97 | 12 | "Language-Team: \n" | ||
98 | 13 | "MIME-Version: 1.0\n" | ||
99 | 14 | "Content-Type: text/plain; charset=UTF-8\n" | ||
100 | 15 | "Content-Transfer-Encoding: \n" | ||
101 | 16 | "Plural-Forms: \n" | ||
102 | 17 | |||
103 | 18 | #. module: sale_project_base | ||
104 | 19 | #: model:ir.model,name:sale_project_base.model_project_project | ||
105 | 20 | #: field:sale.order,related_project_id:0 | ||
106 | 21 | msgid "Project" | ||
107 | 22 | msgstr "" | ||
108 | 23 | |||
109 | 24 | #. module: sale_project_base | ||
110 | 25 | #: model:ir.model,name:sale_project_base.model_sale_order | ||
111 | 26 | msgid "Sales Order" | ||
112 | 27 | msgstr "" | ||
113 | 28 | |||
114 | 29 | #. module: sale_project_base | ||
115 | 30 | #: view:sale.order:0 | ||
116 | 31 | msgid "Create Project" | ||
117 | 32 | msgstr "" | ||
118 | 33 | |||
119 | 34 | #. module: sale_project_base | ||
120 | 35 | #: field:project.project,order_ids:0 | ||
121 | 36 | msgid "Orders" | ||
122 | 37 | msgstr "" | ||
123 | 38 | |||
124 | 0 | 39 | ||
125 | === added file 'sale_project_base/project.py' | |||
126 | --- sale_project_base/project.py 1970-01-01 00:00:00 +0000 | |||
127 | +++ sale_project_base/project.py 2014-03-20 19:06:11 +0000 | |||
128 | @@ -0,0 +1,37 @@ | |||
129 | 1 | # -*- coding: utf-8 -*- | ||
130 | 2 | ############################################################################### | ||
131 | 3 | # | ||
132 | 4 | # Module for OpenERP | ||
133 | 5 | # Copyright (C) 2014 Akretion (http://www.akretion.com). | ||
134 | 6 | # Copyright (C) 2010-2013 Akretion LDTA (<http://www.akretion.com>) | ||
135 | 7 | # @author Sébastien BEAU <sebastien.beau@akretion.com> | ||
136 | 8 | # @author Benoît GUILLOT <benoit.guillot@akretion.com> | ||
137 | 9 | # | ||
138 | 10 | # This program is free software: you can redistribute it and/or modify | ||
139 | 11 | # it under the terms of the GNU Affero General Public License as | ||
140 | 12 | # published by the Free Software Foundation, either version 3 of the | ||
141 | 13 | # License, or (at your option) any later version. | ||
142 | 14 | # | ||
143 | 15 | # This program is distributed in the hope that it will be useful, | ||
144 | 16 | # but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
145 | 17 | # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
146 | 18 | # GNU Affero General Public License for more details. | ||
147 | 19 | # | ||
148 | 20 | # You should have received a copy of the GNU Affero General Public License | ||
149 | 21 | # along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
150 | 22 | # | ||
151 | 23 | ############################################################################### | ||
152 | 24 | |||
153 | 25 | |||
154 | 26 | from openerp.osv import fields, orm | ||
155 | 27 | |||
156 | 28 | |||
157 | 29 | class project_project(orm.Model): | ||
158 | 30 | _inherit = "project.project" | ||
159 | 31 | |||
160 | 32 | _columns = { | ||
161 | 33 | 'order_ids': fields.one2many( | ||
162 | 34 | 'sale.order', | ||
163 | 35 | 'related_project_id', | ||
164 | 36 | 'Orders'), | ||
165 | 37 | } | ||
166 | 0 | 38 | ||
167 | === added file 'sale_project_base/sale.py' | |||
168 | --- sale_project_base/sale.py 1970-01-01 00:00:00 +0000 | |||
169 | +++ sale_project_base/sale.py 2014-03-20 19:06:11 +0000 | |||
170 | @@ -0,0 +1,69 @@ | |||
171 | 1 | # -*- coding: utf-8 -*- | ||
172 | 2 | ############################################################################### | ||
173 | 3 | # | ||
174 | 4 | # Module for OpenERP | ||
175 | 5 | # Copyright (C) 2014 Akretion (http://www.akretion.com). | ||
176 | 6 | # Copyright (C) 2010-2013 Akretion LDTA (<http://www.akretion.com>) | ||
177 | 7 | # @author Sébastien BEAU <sebastien.beau@akretion.com> | ||
178 | 8 | # @author Benoît GUILLOT <benoit.guillot@akretion.com> | ||
179 | 9 | # | ||
180 | 10 | # This program is free software: you can redistribute it and/or modify | ||
181 | 11 | # it under the terms of the GNU Affero General Public License as | ||
182 | 12 | # published by the Free Software Foundation, either version 3 of the | ||
183 | 13 | # License, or (at your option) any later version. | ||
184 | 14 | # | ||
185 | 15 | # This program is distributed in the hope that it will be useful, | ||
186 | 16 | # but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
187 | 17 | # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
188 | 18 | # GNU Affero General Public License for more details. | ||
189 | 19 | # | ||
190 | 20 | # You should have received a copy of the GNU Affero General Public License | ||
191 | 21 | # along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
192 | 22 | # | ||
193 | 23 | ############################################################################### | ||
194 | 24 | |||
195 | 25 | from openerp.osv import fields, orm | ||
196 | 26 | from datetime import date | ||
197 | 27 | |||
198 | 28 | |||
199 | 29 | class sale_order(orm.Model): | ||
200 | 30 | _inherit = "sale.order" | ||
201 | 31 | |||
202 | 32 | _columns = { | ||
203 | 33 | 'related_project_id': fields.many2one( | ||
204 | 34 | 'project.project', | ||
205 | 35 | 'Project', | ||
206 | 36 | readonly=True, | ||
207 | 37 | states={'draft': [('readonly', False)]} | ||
208 | 38 | ), | ||
209 | 39 | } | ||
210 | 40 | |||
211 | 41 | def onchange_related_project_id(self, cr, uid, ids, related_project_id, context=None): | ||
212 | 42 | project_obj = self.pool['project.project'] | ||
213 | 43 | if related_project_id: | ||
214 | 44 | project = project_obj.browse(cr, uid, related_project_id, context=context) | ||
215 | 45 | return {'value': {'project_id': project.analytic_account_id.id}} | ||
216 | 46 | return {} | ||
217 | 47 | |||
218 | 48 | def _prepare_project_vals(self, cr, uid, order, context=None): | ||
219 | 49 | name = u" %s - %s - %s" % ( | ||
220 | 50 | order.partner_id.name, | ||
221 | 51 | date.today().year, | ||
222 | 52 | order.name) | ||
223 | 53 | return { | ||
224 | 54 | 'user_id': order.user_id.id, | ||
225 | 55 | 'name': name, | ||
226 | 56 | 'partner_id': order.partner_id.id, | ||
227 | 57 | } | ||
228 | 58 | |||
229 | 59 | def action_create_project(self, cr, uid, ids, context=None): | ||
230 | 60 | project_obj = self.pool['project.project'] | ||
231 | 61 | for order in self.browse(cr, uid, ids, context=context): | ||
232 | 62 | vals = self._prepare_project_vals(cr, uid, order, context) | ||
233 | 63 | project_id = project_obj.create(cr, uid, vals, context=context) | ||
234 | 64 | project = project_obj.browse(cr, uid, project_id, context=context) | ||
235 | 65 | order.write({ | ||
236 | 66 | 'related_project_id': project_id, | ||
237 | 67 | 'project_id': project.analytic_account_id.id | ||
238 | 68 | }) | ||
239 | 69 | return True | ||
240 | 0 | 70 | ||
241 | === added file 'sale_project_base/sale_view.xml' | |||
242 | --- sale_project_base/sale_view.xml 1970-01-01 00:00:00 +0000 | |||
243 | +++ sale_project_base/sale_view.xml 2014-03-20 19:06:11 +0000 | |||
244 | @@ -0,0 +1,28 @@ | |||
245 | 1 | <?xml version="1.0" encoding="utf-8"?> | ||
246 | 2 | <openerp> | ||
247 | 3 | <data> | ||
248 | 4 | |||
249 | 5 | <record id="view_order_form" model="ir.ui.view"> | ||
250 | 6 | <field name="name">view_project_quotation_order_form</field> | ||
251 | 7 | <field name="model">sale.order</field> | ||
252 | 8 | <field name="inherit_id" ref="sale.view_order_form" /> | ||
253 | 9 | <field name="arch" type="xml"> | ||
254 | 10 | <field name="project_id" position="replace"> | ||
255 | 11 | <field name="project_id" invisible="1" /> | ||
256 | 12 | <group col="4" colspan="2"> | ||
257 | 13 | <field name="related_project_id" | ||
258 | 14 | on_change="onchange_related_project_id(related_project_id, context)" | ||
259 | 15 | domain="[('partner_id','=',partner_id)]" | ||
260 | 16 | attrs="{'readonly':[('partner_id','=', False)]}"/> | ||
261 | 17 | <group attrs="{'invisible':[('related_project_id', '!=', False)]}"> | ||
262 | 18 | <button name="action_create_project" states="draft" | ||
263 | 19 | string="Create Project" | ||
264 | 20 | type="object" icon="gtk-execute" /> | ||
265 | 21 | </group> | ||
266 | 22 | </group> | ||
267 | 23 | </field> | ||
268 | 24 | </field> | ||
269 | 25 | </record> | ||
270 | 26 | |||
271 | 27 | </data> | ||
272 | 28 | </openerp> |
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.