Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[18.0][MIG] sale_order_type: Migration to 18.0 #3352

Open
wants to merge 155 commits into
base: 18.0
Choose a base branch
from
Open

[18.0][MIG] sale_order_type: Migration to 18.0 #3352

wants to merge 155 commits into from
+20,909 −0

Conversation

matiasperalta1
Copy link
Contributor

No description provided.

@rousseldenis
Copy link
Contributor

/ocabot migration sale_order_type

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Oct 17, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Oct 17, 2024
Open
47 tasks
@rousseldenis
Copy link
Contributor

@matiasperalta1 Could you rebase as I've updated pre-commit for v18 ? I suggest you to install it locally before committing

nicolascol
nicolascol approved these changes Oct 17, 2024
View reviewed changes
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.

Functional review OK

@matiasperalta1 matiasperalta1 force-pushed the 18.0-mig-sale_order_type branch 10 times, most recently from 181505a to be54ca1 Compare October 17, 2024 14:47
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@matiasperalta1
Copy link
Contributor Author

@rousseldenis Is it ready to merge?

@matiasperalta1
Copy link
Contributor Author

@pedrobaeza Pedro! Is it ready to merge?

@matiasperalta1 matiasperalta1 force-pushed the 18.0-mig-sale_order_type branch 11 times, most recently from 25beab6 to 408ce7d Compare October 22, 2024 20:37
@lef-adhoc
Copy link

lef-adhoc commented Oct 22, 2024 •
edited
Loading

Hi @pedrobaeza @rousseldenis, what do you think about analytic_account_id no longer exists in the sale.order model? PR
Is it ok to create the field in this module ?

@pedrobaeza
Copy link
Member

Good question @lef-adhoc, and indeed Odoo has removed the duality, only keeping the analytic distribution in the lines, so it doesn't seem correct to keep it in this module, or at least not without extra code that transfer such analytic account to the analytic distribution of each line.

@matiasperalta1
Copy link
Contributor Author

@pedrobaeza @lef-adhoc Good observation. I have already removed everything related to the analytic_account_id field that no longer exists.

pedrobaeza
pedrobaeza reviewed Oct 24, 2024
View reviewed changes
sale_order_type/models/sale_order_type.py
ondelete="restrict",
check_company=True,
)
analytic_account_id = fields.Many2one(
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed as well, isn't it? Anyway, we are stripping out a feature that may be used. The alternative is to put by default this account when adding a line.

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

@pedrobaeza pedrobaeza pedrobaeza left review comments

@lef-adhoc lef-adhoc lef-adhoc approved these changes

@lav-adhoc lav-adhoc lav-adhoc approved these changes

@nicolascol nicolascol nicolascol approved these changes

@rousseldenis rousseldenis rousseldenis approved these changes

Assignees
No one assigned
Labels
approved migration needs review ready to merge
Projects
None yet
Milestone
18.0
Development

Successfully merging this pull request may close these issues.