SmsResponseMessage.cs is missing parameter.

Vonage / vonage-dotnet-sdk

Vonage REST API client for .NET, written in C#. API support for SMS, Voice, Text-to-Speech, Numbers, Verify (2FA) and more.

https://developer.vonage.com/

Apache License 2.0

106 stars 86 forks source link

SmsResponseMessage.cs is missing parameter. #577

Closed fahadhabib80 closed 5 months ago

fahadhabib80 commented 6 months ago

Bug Description In SmsResponseMessage.cs class, there should be AccountRef and ClientRef properties. However Client Ref is missing and Account Ref comments are mentioned to have client-ref value, which is incorrect. This bug means we cannot match Sms Response with original request in async situation as once a response came back, we dont know which original request it belongs to. Hence client-ref match is needed.

Expected behaviour New property ClientRef needs to be added. See screenshot.

Screenshots client-ref

Code snippets n/a

Environment(please complete the following information): v7.2.0 SDK release

Additional context none

fahadhabib80 commented 6 months ago

Vonage API ref https://developer.vonage.com/en/api/sms?source=messaging

client-ref2

Tr00d commented 6 months ago

Hi @fahadhabib80,

Thanks for raising this one out. I find it odd that this property is missing because that API didn't change in a while. It's actually the legacy API to send SMS; we always recommend customers to favor MessagesAPI instead.

I'll get back to you as soon as there's progress on this.

fahadhabib80 commented 6 months ago

Hi @Tr00d

Thanks for picking it up. I did not realise that Message API is also for SMS. I looked and picked SMS API by default. As dev is still actively done on this, so assumption is that it will supported in near future. I could switch to Message API if that's the direction of travel from Vonage

Thanks

Tr00d commented 6 months ago

Messages API supports multiple carriers, including SMS. Don't get me wrong - SMS API is going to stay for a long time but we tend to recommend people to use Messages, if possible, for multiple reasons:

SMS API is considered legacy as it still only supports Basic Auth, while Messages supports Bearer Auth.
Messages API is the one receiving the most updates and upgrades.

Tr00d commented 6 months ago

Hi @fahadhabib80 ,

This has been fixed in the latest version, v7.2.2.

Let me know if it works out for you.

Tr00d commented 5 months ago

Closing issue after two weeks. Don't hesitate to reach out if yoou face any problem.

fahadhabib80 commented 5 months ago

Apologies for not making any comments here. It worked well for my use case.

Thanks for quick fix.