Add first upsert methods for Deals and Contact#83
Add first upsert methods for Deals and Contact#83redroot wants to merge 9 commits intozendesk:masterfrom
Conversation
| validate_type!(deal) | ||
|
|
||
| attributes = sanitize(deal) | ||
| query_string = URI.encode_www_form(filters) |
There was a problem hiding this comment.
i was trying to find a way on encoding the hash without using Rails only methods (like Object.to_query) or a new gem like Addressable, this seems to do the trick, but other opinions welcome
spec/services/deals_service_spec.rb
Outdated
| it 'calls the upsert route with encoded filters' do | ||
| filters = { name: 'unique_name', 'custom_fields[external_id]': 'unique-1' } | ||
| attributes = filters.merge('custom_fields[category]': 'bags') | ||
| expect(client.deals.upsert(filters, attributes)).to be_instance_of BaseCRM::Deal |
There was a problem hiding this comment.
these tests seems to require an Access Token and actually attempt to hit a running API, is that expected?
redroot
changed the title
|
|
||
| attributes = sanitize(deal) | ||
| query_string = URI.encode_www_form(filters) | ||
| _, _, root = @client.post("/contacts/upsert?#{query_string}", attributes) |
There was a problem hiding this comment.
💡 If the intention of upsert is to be idempotent, then you should use a PUT.
The difference between PUT and POST is that PUT is idempotent: calling it once or several times successively has the same effect (that is no side effect), where successive identical POST may have additional effects, like passing an order several times.
There was a problem hiding this comment.
That may be so, but according to the docs it Zendesk Sell's case its a POST https://developers.getbase.com/docs/rest/reference/deals
| filters = { last_name: 'unique_name', 'custom_fields[external_id]': 'unique-1' } | ||
| contact = create(:contact, last_name: 'unique_name', custom_fields: { external_id: 'unique-1'}) | ||
| expect(client.contacts.upsert(filters, contact)).to be_instance_of BaseCRM::Contact | ||
| end |
There was a problem hiding this comment.
🤔 This test seems to cover the "update" path, but we are missing the "insert" path.
If the endpoint does an upsert both scenarios should be tested
There was a problem hiding this comment.
Agreed, waiting on some guidance on the test suite in general here #83 (comment) before i improve this but i agree
| filters = { name: 'unique_name', 'custom_fields[external_id]': 'unique-1' } | ||
| deal = create(:deal, name: 'unique_name', custom_fields: { external_id: 'unique-1'}) | ||
| expect(client.deals.upsert(filters, deal)).to be_instance_of BaseCRM::Deal | ||
| end |
Co-authored-by: Emiliano Mancuso <emiliano.mancuso@carwow.co.uk>
Co-authored-by: Emiliano Mancuso <emiliano.mancuso@carwow.co.uk>
Co-authored-by: Emiliano Mancuso <emiliano.mancuso@carwow.co.uk>
|
@struniu hi! any chance of a review / update on whether this gem is maintained still? |
2 participants
Adds the ability to send a POST to /deals/upsert as specified here https://developers.getbase.com/docs/rest/reference/deals and equivalent for contacts