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

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.

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

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.

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).

@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?

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

Daniel suggestions are good for me too.

Regards.

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?

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.

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

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
-....

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

Regards.

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