Skip to content

Conversation

@matiasperalta1
Copy link
Contributor

No description provided.

@matiasperalta1 matiasperalta1 force-pushed the 18.0-t-43366-mnp branch 3 times, most recently from 9ed0a91 to b3085ac Compare December 18, 2024 14:42
Copy link

@nicolascol nicolascol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nicolascol
Copy link

Hi @rvalyi !
Can you merge this PR?
We added the "project_id" because of the new analytic approach in v18.
Odoo PR with new approach

@rousseldenis
Copy link
Contributor

@matiasperalta1 Could you rename your commit and this description as this is more correct: [IMP] sale_order_type: Use project instead of analytic account on types.

But, with this change, existing installs will loose data. It should contain migration scripts if possible.

Copy link
Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, IMHO, this is not correct as analytic account still exists.

This is a change of behavior as an analytic account is not a project. That's why you've added a new dependency from sale_project.

If you want to assign a project to the type, you should do a new module that allow that.

@nicolascol
Copy link

Hi @rousseldenis
The "analytic_account_id" field doesn't exist in sale.order v18, Odoo replaced it with "project_id". More info
This PR was made to adapt sale_order_type to v18. In sale_order_type v17 an "analytic_account_id" could be configured. In the v18 migration PR we were evaluating this change to adapt it but that PR was merged and that is why we are now making this new PR.

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a migration script to assign the project with the account_id matching the original analytic_account_id values on the order types?

@matiasperalta1 matiasperalta1 force-pushed the 18.0-t-43366-mnp branch 2 times, most recently from f9c4571 to b206df8 Compare October 20, 2025 16:45
@rousseldenis
Copy link
Contributor

@matiasperalta1 Could you fix tests ?

@jjscarafia
Copy link
Contributor

@rousseldenis @StefanRijnhart @matiasperalta1
As I understand:

  1. the analytic_account_id is not used anymore, could be removed

  2. if we want to add some new functionality related to "analytics" maybe we should add :
    2a. add an "analytic_distribution" field (I believe this would be cleaner)
    2b. or extends analytic distribution models to be able to map with types

  3. If there is need to set "project_id" by a sale order type (not linked to analytics need), I agree that a new module would be needed

What do you think of going with "1" and "2a"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants