-
-
Notifications
You must be signed in to change notification settings - Fork 385
[16.0] [FIX] delivery_dropoff_site: name search in fields pointing to res.partner is broken #1048
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
[16.0] [FIX] delivery_dropoff_site: name search in fields pointing to res.partner is broken #1048
Conversation
@BhaveshHeliconia check this out. It's affecting your migration to 17.0 as well. |
You were involved in migration to 16.0. Would you be so kind to review this @jdoutreloux @ThomasBinsfeld @DorianMAG @rousseldenis @gaelTorrecillas @flotho. |
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
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 |
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.
@rrebollo thanks for the fix. LGTM!
@rousseldenis can we proceed with merging this? |
@pedrobaeza 🙏 I was hoping not to bother you with this, but... could you please merge it? 🚀 |
/ocabot merge patch |
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at 3c4b76f. Thanks a lot for contributing to OCA. ❤️ |
What is wrong?
☝️ is recorded on runboat.
The issue comes from this code:
delivery-carrier/delivery_dropoff_site/models/res_partner.py
Line 14 in 63ae7c7
What’s done
Following the approach used in other OCA addons,
_rec_names_search
is now properly set, and everything works as expected.18.0 isn't been affected. 17.0 hasn't been merged yet (#951) but it is affected.
EDIT
Given that the coverage check was failing, we addressed that as well. Inspired by 18.0 tests.
@BinhexTeam