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.

Sign up for GitHub

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

Jump to bottom

[16.0][ADD] crm_partner_contact_role #588

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from
Open

[16.0][ADD] crm_partner_contact_role #588

wants to merge 1 commit into from

Conversation

AungKoKoLin1997
Copy link

@AungKoKoLin1997 AungKoKoLin1997 commented Jul 23, 2024 •
edited
Loading

This PR adds role_ids to the crm.lead model and propagates them to the res.partner record when it is created from crm.lead.

Related: OCA/partner-contact#1812
@qrtl QT4695

@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 16.0-add-crm_partner_contact_role branch from b5e74ca to 69bbce7 Compare August 9, 2024 03:09
damdam-s
damdam-s approved these changes Sep 30, 2024
View reviewed changes
Copy link
Member

@damdam-s damdam-s left a comment

Choose a reason for hiding this comment

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

lgtm

yostashiro
yostashiro reviewed Oct 7, 2024
View reviewed changes
crm_partner_contact_role/models/crm_lead.py Outdated

role_ids = fields.Many2many(
comodel_name="res.partner.role",
compute="_compute_role",
Copy link
Member

Choose a reason for hiding this comment

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

Would there be an issue if we just made it a related field?

Copy link
Author

Choose a reason for hiding this comment

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

Yes. It will when we want to create new customer with filling role_ids and contact name from pipeline without assigning partner_id.

@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 16.0-add-crm_partner_contact_role branch from 69bbce7 to 6e79b17 Compare October 11, 2024 02:17
yostashiro
yostashiro approved these changes Oct 11, 2024
View reviewed changes
Copy link
Member

@yostashiro yostashiro left a comment

Choose a reason for hiding this comment

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

Minor suggestions, otherwise LGTM.

crm_partner_contact_role/tests/test_crm_partner_contact_role.py
self.assertTrue(bool(partner), "Partner was not created")
self.assertEqual(
partner.role_ids.ids,
lead.role_ids.ids,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
lead.role_ids.ids,
[self.role_1.id, self.role_2.id],

crm_partner_contact_role/tests/test_crm_partner_contact_role.py
)
self.assertEqual(
lead.role_ids.ids,
partner.role_ids.ids,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
partner.role_ids.ids,
[self.role_1.id, self.role_2.id],

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

2 similar comments
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@AungKoKoLin1997
Copy link
Author

@oca/crm-maintainers
Is it possible to take a look at this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damdam-s damdam-s damdam-s approved these changes

@yostashiro yostashiro yostashiro approved these changes

Assignees
No one assigned
Labels
approved ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AungKoKoLin1997 @OCA-git-bot @yostashiro @damdam-s