-
-
Notifications
You must be signed in to change notification settings - Fork 948
[17.0][FW][IMP] partner_contact_address_default: add option to allow selecting any partner #2221
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][FW][IMP] partner_contact_address_default: add option to allow selecting any partner #2221
Conversation
|
I'm somehow getting errors on the partner fields (
@AungKoKoLin1997 Can you please check and see if you can tell anything? |
|
|
@yostashiro partner-contact/partner_stage_only_confirmed/models/base.py Lines 31 to 36 in 9c7b0d1
I tried to fix with that approach. |
7e4f184 to
d815ca6
Compare
d815ca6 to
44d6294
Compare
yostashiro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review. LGTM. Although #2229 needs to be merged, it shouldn't block this one.
edescalona
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
This PR has the |
|
Hi @AungKoKoLin1997, great work! Just one suggestion regarding shipping addresses: Thanks |
…s to display delivery addresses and partner itself
edescalona
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
|
/ocabot merge major |
|
On my way to merge this fine PR! |
|
@NL66278 your merge command was aborted due to failed check(s), which you can inspect on this commit of 17.0-ocabot-merge-pr-2221-by-NL66278-bump-major. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
|
/ocabot merge major |
|
What a great day to merge this nice PR. Let's do it! |
|
Congratulations, your PR was merged at dc4a370. Thanks a lot for contributing to OCA. ❤️ |
@edescalona I was considering in what context or business operation it would be necessary to restrict selection to only delivery address contacts and the partner itself. With the existing option that allows selecting any partner, we can already choose both the delivery address contact and the partner itself—so this case is already covered. Also, with the module’s native behavior, we can already select only related delivery addresses. If you agree, I’d like to revert my latest commit, since the existing features already cover this case. Am I missing anything? |

Forward port of #2187
This PR adds an option to allow selecting any partner for default addresses, instead of restricting selections to child contacts.
@qrtl QT5088