Skip to content

[18.0] [MIG] account_banking_mandate_sale_contact: Migration to 18.0 #1465

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

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

Conversation

bizzappdev
Copy link

Dependency PR for module account_banking_mandate_sale #1456

@xaviedoanhduy
Copy link
Contributor

hello @bizzappdev, do you have any plans to continue this work?

If you have any problems, I am here to help you.

@bizzappdev
Copy link
Author

hello @bizzappdev, do you have any plans to continue this work?

If you have any problems, I am here to help you.

@xaviedoanhduy Yes, we are planning to continue the work. The module is ready, but for some strange reason (it might be a dependent module from test-requirement.txt), it is causing the test cases to fail. We are investigating in depth.

@xaviedoanhduy
Copy link
Contributor

xaviedoanhduy commented May 16, 2025

as you said, it happens in 2 modules account_payment_mode and account_payment_sale, have a look in the PR that I tried to fix it: xaviedoanhduy#4

I think when this PR #1467 is merged the CI will be green when you rebase with the original branch

_inherit = "sale.order"

@api.depends(
"partner_id", "partner_invoice_id", "partner_shipping_id", "payment_mode_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 copy all the dependent fields in this decorator, just add new fields that don't appear in the original function and the ORM will automatically join them

Copy link
Author

Choose a reason for hiding this comment

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

Done

@bizzappdev bizzappdev force-pushed the 18.0-mig-account_banking_mandate_sale_contact-BAD branch from 453d99e to 68fefb6 Compare May 16, 2025 12:11
@xaviedoanhduy
Copy link
Contributor

hello @bizzappdev, #1467 has been merged, is this PR ready?

@bizzappdev bizzappdev force-pushed the 18.0-mig-account_banking_mandate_sale_contact-BAD branch from 68fefb6 to 93b12ee Compare May 19, 2025 05:01
@bizzappdev bizzappdev marked this pull request as ready for review May 19, 2025 08:00
@bizzappdev
Copy link
Author

@xaviedoanhduy Yes, it is now ready for review, but the dependency Module migration PR is still there, which will restrict merging this PR.

Copy link
Contributor

@xaviedoanhduy xaviedoanhduy left a comment

Choose a reason for hiding this comment

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

Thanks for the work on this! The proposed changes make sense and look good to me overall 👍

Approved ✅

help="The partner of the sales in which odoo will search for the mandate"
title="The contact of this company in which odoo will search for the mandate on sales. 
-Customer Mandate: Odoo will look the mandate in the sale partner, whether is an individual or the company. 
-Commercial Customer Mandate: Odoo will look the mandate in the sale partner company. 
-Invoice Address Mandate: Odoo will look the mandate in the sale invoice address. 
-Delivery Address Mandate: Odoo will look the mandate in the sale delivery address. 
-False: Odoo will use the first mandate he founds for the partner company. Odoo will also use this option if no default mandate is found in the partner of the above options"
>
<field name="sale_default_mandate_contact" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<field name="sale_default_mandate_contact" />
<field name="sale_default_mandate_contact" class="oe_inline" />

before

image

after

image

<field name="model">res.partner</field>
<field
name="inherit_id"
ref="account_payment_partner.view_partner_property_form"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ref="account_payment_partner.view_partner_property_form"
ref="account_banking_mandate_contact.view_partner_property_form"

I think this will look clearer

Copy link
Author

Choose a reason for hiding this comment

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

Both changes are applied.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks

@bizzappdev bizzappdev force-pushed the 18.0-mig-account_banking_mandate_sale_contact-BAD branch from 93b12ee to 8fc7933 Compare May 22, 2025 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants