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

[17.0] [MIG] sale_order_line_delivery_state: Migration to 17.0 #3333

Open
wants to merge 16 commits into
base: 17.0
Choose a base branch
from

Conversation

bizzappdev
Copy link
Contributor

No description provided.

@bizzappdev bizzappdev force-pushed the 17.0-mig-sale_order_line_delivery_state-BAD-RUS branch from eac4ac9 to 9ebef89 Compare September 28, 2024 12:57
@bizzappdev bizzappdev force-pushed the 17.0-mig-sale_order_line_delivery_state-BAD-RUS branch from 9ebef89 to b2daa7c Compare September 28, 2024 13:44
@bizzappdev bizzappdev marked this pull request as ready for review September 29, 2024 15:07
@yvaucher
Copy link
Member

/ocabot migration sale_order_line_delivery_state

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Sep 30, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Sep 30, 2024
89 tasks
@@ -18,14 +18,14 @@
type="object"
string="Force delivery done"
states="done"
attrs="{'invisible': ['|', ('force_delivery_state', '=', True), ('delivery_state', '=', 'done')]}"
invisible="['|', ('force_delivery_state', '=', True), ('delivery_state', '=', 'done')]"
Copy link
Member

Choose a reason for hiding this comment

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

Does this work? I thought a Python expression was expected here, like force_delivery_state or delivery_state == 'done'

],
# Compute method have a different name then the field because
# the method _compute_delivery_state already exist to compute
# the field delivery_set in odoo delivery module
Copy link
Member

Choose a reason for hiding this comment

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

In Odoo 17, this no longer seems to be the case. There is a _compute_delivery_state method, still in the Odoo delivery module, but it's only on sale.order, not on sale.order.line.

@bizzappdev bizzappdev force-pushed the 17.0-mig-sale_order_line_delivery_state-BAD-RUS branch from b2daa7c to ce4f543 Compare October 9, 2024 11:00
@bizzappdev bizzappdev force-pushed the 17.0-mig-sale_order_line_delivery_state-BAD-RUS branch from ce4f543 to b703ec0 Compare October 9, 2024 12:18
Comment on lines +20 to +21
states="done"
invisible="force_delivery_state or state == 'done'"
Copy link
Member

Choose a reason for hiding this comment

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

Ah there is a states attribute as well. This might explain why you did not use the delivery_state field from the previous attrs, even though it still makes sense.

Suggested change
states="done"
invisible="force_delivery_state or state == 'done'"
invisible="force_delivery_state or state != 'done' or delivery_state == 'done'"

Comment on lines +27 to +28
states="done"
invisible="not force_delivery_state"
Copy link
Member

Choose a reason for hiding this comment

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

Here I don't think it makes sense to hide the button if the order is not in done state. If force_delivery_state is set on an order that is not in done state it's an exceptional case that should be visualized IMHO.

Suggested change
states="done"
invisible="not force_delivery_state"
invisible="not force_delivery_state"

store=True,
)

force_delivery_state = fields.Boolean(
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this field should be set to copy=False. What do you think?

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.