novuhq / novu-ruby

Ruby SDK for Novu - The open-source notification infrastructure for engineers. 🚀
https://rubygems.org/gems/novu
MIT License
23 stars 10 forks source link

Bulk event trigger method having problem #37

Open amirali-ashraf opened 11 months ago

amirali-ashraf commented 11 months ago

When I try to use this method:

client = Novu::Client.new(<API-KEY>)
client.trigger_bulk_event(
  events: [
    {name: 'name', to: {subscriberId: 'asdfasdf'}, payload: {message: 'lorem'}},
    {name: 'name', to: {subscriberId: 'sdfgsdfg'}, payload: {message: 'lorem'}},
  ]
)

I am getting the following error:

{"data"=>[{"status"=>"error", "error"=>["identifier must be a string"], "acknowledged"=>true}]}

I noticed that there is an issue with the HTTParty when you want to submit an array as a body. It does not parse it properly.

https://github.com/jnunemaker/httparty/issues/719

I can help to address the issue if we can agree on a solution.

My suggestion is to make add headers: { 'Content-Type' => 'application/json' } to all or post requests and also, convert post body to_json. I think that is the only work around.

The harder solution is to use Faraday instead of HTTParty which is maintained better.

Thanks

unicodeveloper commented 11 months ago

The value of subscriberId should be a string @amirali-ashraf

amirali-ashraf commented 11 months ago

The value of subscriberId should be a string @amirali-ashraf

Hi, I tried it by all means, and actually I am using string for all parts which is not working. I can write a quick spec test which can give you a better insight.

I updated my main code as well to avoid further misunderstandings.

Thanks

unicodeveloper commented 11 months ago

Please can I see your code snippet?

amirali-ashraf commented 11 months ago

Of course!

require "bundler/inline"

gemfile(true) do
  ruby "3.1.2"
  gem "novu"
end

client = Novu::Client.new('YOUR-API-KEY')

res = client.create_subscriber({subscriberId: 'ghgh456fthy45633efwf324r', email: 'test@test.com'})
puts res

res = client.create_subscriber({subscriberId: '2fwef234rwef4teghrt7657h', email: 'test1@test.com'})
puts res

res = client.trigger_bulk_event({events: [
  {name: 'in-app', to: {subscriberId: 'ghgh456fthy45633efwf324r'}, payload: {message: 'lorem'}},
  {name: 'in-app', to: {subscriberId: '2fwef234rwef4teghrt7657h'}, payload: {message: 'lorem'}},
]})
puts res

You can adjust your ruby version, btw.

But if you use to_json then would work:

require "bundler/inline"

gemfile(true) do
  ruby "3.1.2"
  gem "novu"
end

client = Novu::Client.new('your-api-key')

res = client.create_subscriber({subscriberId: 'ghgh456fthy45633efwf324r', email: 'test@test.com'})
puts res

res = client.create_subscriber({subscriberId: '2fwef234rwef4teghrt7657h', email: 'test1@test.com'})
puts res

res = client.post('/events/trigger/bulk', body: {events: [
  {name: 'in-app', to: {subscriberId: 'ghgh456fthy45633efwf324r'}, payload: {message: 'lorem'}},
  {name: 'in-app', to: {subscriberId: '2fwef234rwef4teghrt7657h'}, payload: {message: 'lorem'}},
]}.to_json, headers: { 'Content-Type' => 'application/json'})
puts res
Eazybright commented 11 months ago

@amirali-ashraf I guess you will have to use .to_json for now.

If you check the spec/events_spec test file, .to_json was added to the body parameter as well.

amirali-ashraf commented 11 months ago

@Eazybright yeah I tried that one too, but when you use .to_json actually ruby changes it to string formatted json, which then needs to be accompanied with content-type: json, but the function does not allow extra params like options. so the only work around is to use the post method directly.

Eazybright commented 11 months ago

@amirali-ashraf I understand your point of view now. I would love to see your PR to resolve this issue with the current httpparty that's being used.

amirali-ashraf commented 11 months ago

@Eazybright Hey, thanks for you reply. My only workaround this issue is to post by converting the hash into json and set the header content to json where you post the data. Does that work for you?

Also, I think this would not be the only instance that you are sending arrays. Can you tell me if there is so we can fix them in one shot or you want them to be fixed in future?

Eazybright commented 11 months ago

that's the only instance for now

Eazybright commented 11 months ago

@amirali-ashraf i had the same issue with the bulk subscriber creation API. You can check my PR for the fix https://github.com/novuhq/novu-ruby/pull/39

amirali-ashraf commented 11 months ago

raised PR #45

Eazybright commented 11 months ago

raised PR #45

@unicodeveloper

amirali-ashraf commented 9 months ago

@unicodeveloper Hi, could you please confirm when will be this part available on a new release?