-
Notifications
You must be signed in to change notification settings - Fork 68
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
Add client.merge_leads #20
Conversation
Remove params init Fix array join
a312d2d
to
a816afa
Compare
@raszi added merge leads method, do i need to show some real requests/responses as part of the PR, i followed the API docs, and also tried with my test marketo account |
Any chance in looking at this? |
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.
Thank you!
lib/mrkt/concerns/crud_leads.rb
Outdated
params[:leadIds] = losing_lead_ids.join(",") if losing_lead_ids | ||
params[:mergeInCRM] = merge_in_crm | ||
|
||
post("/rest/v1/leads/#{winning_lead_id}/merge.json?#{params.to_query}") do |req| |
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.
I believe this should go to the body.
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.
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.
Oh, sorry, I did not catch that. Then we should use the query.
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.
@raszi cool no worries!
@@ -40,5 +40,17 @@ def associate_lead(id, cookie) | |||
json_payload(req, {}) | |||
end | |||
end | |||
|
|||
def merge_leads(winning_lead_id, losing_lead_ids, merge_in_crm: false) | |||
params = Faraday::Utils::ParamsHash.new |
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.
There is no need to use Faraday::Utils::ParamsHash.new
a simple Hash would make it.
lib/mrkt/concerns/crud_leads.rb
Outdated
json_payload(req, {}) | ||
end | ||
end | ||
|
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.
Please remove the unnecessary newline.
spec/concerns/crud_leads_spec.rb
Outdated
end | ||
end | ||
end | ||
|
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.
Please remove the unnecessary newline from here.
spec/concerns/crud_leads_spec.rb
Outdated
|
||
describe '#merge_leads' do | ||
let(:id) { 1 } | ||
let(:losing_lead_ids){[2,3,4]} |
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.
Please format it like this:
let(:losing_lead_ids) { [2, 3, 4] }
lib/mrkt/concerns/crud_leads.rb
Outdated
def merge_leads(winning_lead_id, losing_lead_ids, merge_in_crm: false) | ||
params = Faraday::Utils::ParamsHash.new | ||
|
||
params[:leadIds] = losing_lead_ids.join(",") if losing_lead_ids |
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.
Please use simple quotes when interpolation is not necessary.
No description provided.