search

jonallured / braze_ruby

A wrapper gem for the Braze REST API.
MIT License
18 stars 21 forks source link

Add support for deleting scheduled triggers #47

Closed dgalarza closed 2 years ago

dgalarza commented 2 years ago

https://www.braze.com/docs/api/endpoints/messaging/schedule_messages/post_delete_scheduled_triggered_messages/

jonallured commented 2 years ago

Hi @dgalarza thanks for the PR!!

I was reviewing the diff and the first thing that stood out to me was the difference between the delete_campaign_trigger_schedule added here and the existing trigger_campaign_send and trigger_campaign_schedule methods. The pattern seems to be to init a BrazeRuby::REST::Base subclass and then call perform on it.

There are three subclasses of BrazeRuby::REST::Base for each of these methods:

And they don't all behave the same way - the first sends the payload when calling perform and the remaining two send it when making the instance. My immediate thought was: is this consistent elsewhere?

So then I cruised through the other BrazeRuby::REST::Base subclasses and wow it is NOT consistent. 😢

Anyway, this PR should not have the responsibility of making these subclasses consistent so I'm happy to merge and move on with my life but I wanted to note this because it makes my anxiety more public, haha.

dgalarza commented 2 years ago

Haha @jonallured no problem at all! Not gonna lie, I was trying to figure out which convention to follow and ended up picking one. Figured it could always be changed if there is a desired one to move forward with.

Thanks!