getonbrd / pipedrive-connect

Ruby library for the Pipedrive API.
MIT License
19 stars 30 forks source link

Add ability to add or remove Participants from a Deal #34

Closed emilioeduardob closed 1 year ago

emilioeduardob commented 1 year ago

This PRs modifies the has_many method to add new_{relation} and delete_{relation}

For example:

# Defining participants on a Deal class
has_many :participants

# it will generate
deal.new_participant(person_id: person.id)

# removing a participant
deal.delete_participant(participant)

But this will also generate the same methods for other has_many associations, like for Followers, but it's not useful for has_many :products for instance

If you guys prefer, I can just add these methods to handle participants as methods directly on the Deal resource

PS: I'll add specs after the first review of the PR

j4rs commented 1 year ago

Hello Emilio,

Thanks for the contribution. QQ: Why isn't useful for products? I see the API doc includes endpoints for adding or deleting a product from a deal.

emilioeduardob commented 1 year ago

oh, you are right, I guess I was confusing the existing add_product method that receives a Product instance Yes, it should work ok with products, in this case, deal will support add_product with Product resource instance and new_product with a regular hash with params

j4rs commented 1 year ago

With your solution, I suggest naming the methods add_* instead of new_* deprecating the current add_product, and documenting the breaking change - for the particular case of Deal - in the changelog.

emilioeduardob commented 1 year ago

Hey @j4rs , I've updated the PR with your suggestions. Let me know if you need me to make any other adjustment