Vonage / vonage-ruby-sdk

Vonage REST API client for Ruby. API support for SMS, Voice, Text-to-Speech, Numbers, Verify (2FA) and more.
https://developer.vonage.com
Apache License 2.0
217 stars 108 forks source link

character ç should not be part of GSM7 character set #255

Closed loicginoux closed 1 year ago

loicginoux commented 1 year ago

Bug: Trying to send an sms with a text like commerçant. In our app, depending on the content of the text we encode the sms in text or unicode. but because Vonage::GSM7::CHARACTERS.chars contains the ç, and we base our logique about encoding around this constant, it sends the SMS encoded in text instead of unicode and the sms is received as commer?ant.

proposed fix: According to this page https://www.world-text.com/docs/gsm-character-set.php the ç is not in the GSM 7 character set and the Ç should be in it so the fix would be to change the list:

Vonage::GSM7::CHARACTERS.chars = Vonage::GSM7::CHARACTERS.chars - ["ç"] + ["Ç"]

With this change the sms is correctly received because encoding is `unicode

superchilled commented 1 year ago

Thanks for raising this @loicginoux. I'll look into it.

superchilled commented 1 year ago

Hi @loicginoux. This is fixed in the latest release: https://github.com/Vonage/vonage-ruby-sdk/releases/tag/v7.8.2

As a side-note, I wouldn't necessarily recommend building application logic around this. In the SDK, the GSM7::encoded method is only used to log a warning when sending a SMS that contains non-GSM7 chars and unicode hasn't been selected. It isn't really intended to have other logic built on top of it.

loicginoux commented 1 year ago

Thank you for the release. About not using this for our logic, the thing is that we noticed that many SMS we send are split into multiple messages, even when this is not necessary. According to vonage documentation, we should choose between the text and unicode encoding based on the content of the message we need to send. Always using unicode double the "parts" needed for a message which could have been sent by text. So in order to save money we need to send sms encoded in 'text' when there are no special characters in the SMS content.

superchilled commented 1 year ago

Always using unicode double the "parts" needed for a message which could have been sent by text. So in order to save money we need to send sms encoded in 'text' when there are no special characters in the SMS content.

Yes this is true, if you set the type to unicode the API will use 2 'spaces' per character, so its definitely a good idea to select text if the message only contains GSM7 characters (especially for long messages that require multiple parts). I guess my point was that the intention of the GSM::encoded method isn't to automatically make the decision for you which type to set, but only to flag a warning. If you do want to use it to automatically set type, of course you ca use the SDK in any way you want, I just want to be clear that this is not the intended purpose of the method (because, as you point out, there is a cost implication of setting one either unicode or text, we don't want to automatically make that decision for users of the SDK, but ultimately leave that decision up to the user).