intercom / intercom-go

Go Bindings For Intercom
https://developers.intercom.io/reference
Other
70 stars 78 forks source link

Webhook Notifications #75

Closed heldtogether closed 6 years ago

heldtogether commented 7 years ago

This adds the ability to parse a notification from json and populates structs of the correct type

choran commented 6 years ago

Hi @heldtogether, I was just taking a look at this but I wasnt clear on what you were trying to do. Do you want to be able to subscribe to Intercom webhook notifications? e.g. like this in Ruby? I saw your example in the readme but if you could provide more context of example use case that would be great. Just make sure I fully understand exactly what you are trying to do. Thanks again Cathal

heldtogether commented 6 years ago

@choran this functionality assumes that you have already received a webhook notification. It decodes that notification into structs of the correct type so that you can do things with the models. Responsibility for subscribing to notifications is handled elsewhere. This just lets you get proper objects from the JSON payload.

choran commented 6 years ago

@heldtogether ah, ok .. I get your now. I think we have something similar in our Java SDK but it is more lightweight. I do see its usefulness, I will just take another look at it now that you have clarified it. thanks cathal

heldtogether commented 6 years ago

@choran any update on this?

choran commented 6 years ago

Hi @heldtogether Just taking another look at it now and will update shortly

choran commented 6 years ago

@heldtogether I see that you are not covering all webhook topics, for example you are not covering leads right, I guess in that case you will just return the raw data in the notification?

heldtogether commented 6 years ago

@choran I think I'm covering the topics listed in the docs (https://developers.intercom.com/v2.0/reference#topics), but if any others are received then the RawData field is still populated with the json which can be decoded as required.

choran commented 6 years ago

@heldtogether the only ones I noticed were the contacts related notifications, the contacts are leads (the endpoint is still called contacts so it can be a little confusing) But I think thats ok since like you said RawData would catch it anyway. Since its new functionality i.e. we dont have webhook notifications in other SDK other than Java I just need to get one more review internally here. Should be able to merge then Thanks again Cathal