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.

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

[ADD] sale_stock_partner_warehouse #3044

Merged

Conversation

santostelmo
Copy link
Contributor

Description
Allow to choose by default a warehouse on SO based on a Partner parameter.
If the warehouse parameter is completed, then this value would be the default one on the sale.
If empty, then the warehouse on sale would be the standard default warehouse.

],
"depends": [
"sale_stock",
"delivery",
Copy link
Contributor

Choose a reason for hiding this comment

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

why delivery?
If it's only for the view, you can simply use <group name="sale" position="inside">.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed dependency

_inherit = "sale.order"

warehouse_id = fields.Many2one(
compute="_compute_warehouse_id", store=True, precompute=True
Copy link
Contributor

Choose a reason for hiding this comment

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

why precompute?

Copy link
Contributor Author

@santostelmo santostelmo Mar 28, 2024

Choose a reason for hiding this comment

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

To compute the value before the record creation

Copy link
Contributor

Choose a reason for hiding this comment

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

@santostelmo As warehouse_id is defined in sale_stock, you don't need to repeat the properties. Just override _compute_warehouse_id()

Copy link
Contributor

Choose a reason for hiding this comment

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

@santostelmo I know, I mean... why do you need that? I guess it's because the wh is supposed to be already there on create.
If there's a motivation, it's worth to add a comment there.

@santostelmo santostelmo force-pushed the 16.0-add-sale_stock_partner_warehouse branch from d3e90d7 to 6932b67 Compare March 28, 2024 09:23
Copy link

@luisDIXMIT luisDIXMIT left a comment

Choose a reason for hiding this comment

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

LGTM!!

class ResPartner(models.Model):
_inherit = "res.partner"

sale_warehouse_id = fields.Many2one("stock.warehouse", string="Sale Warehouse")
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO, this should be company dependent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. The company of the partner, if defined must be consistent with the one set in the warehouse

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually after thinking about that I think we don't want to have sale_warehouse_id dependent of the company. We want to be able to select any warehouse from active companies.
With that option a Sale can be created in a company A and Delivered from the Warehouse of the company B

compute="_compute_warehouse_id", store=True, precompute=True
)

@api.depends("user_id", "company_id", "partner_id")
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to repeat user_id and company_id. Odoo will append partner_id to depends list

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you I updated that

Copy link
Contributor

@Jaimermaccione Jaimermaccione 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 on runboat! LGTM!

@rousseldenis
Copy link
Contributor

@simahawk Could you update your review ?

@santostelmo santostelmo force-pushed the 16.0-add-sale_stock_partner_warehouse branch from 6932b67 to 4d0cea5 Compare June 24, 2024 09:29
@rousseldenis
Copy link
Contributor

@simahawk

Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

Minor remark, LG otherwise

_inherit = "sale.order"

warehouse_id = fields.Many2one(
compute="_compute_warehouse_id", store=True, precompute=True
Copy link
Contributor

Choose a reason for hiding this comment

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

@santostelmo I know, I mean... why do you need that? I guess it's because the wh is supposed to be already there on create.
If there's a motivation, it's worth to add a comment there.

@HaraldPanten
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-3044-by-HaraldPanten-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 2317798 into OCA:16.0 Sep 19, 2024
8 of 9 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 1d4ae62. Thanks a lot for contributing to OCA. ❤️

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.

7 participants