Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: SSML tag name capitalised issue #212

Closed
wants to merge 1 commit into from
Closed

fix: SSML tag name capitalised issue #212

wants to merge 1 commit into from

Conversation

abhishek-eltropy
Copy link

Fixes

#207

Currently, in twilio go sdk, Ssml tags are capitalized causing error in TwiML apps.

Warning [12200](https://www.twilio.com/docs/api/errors/12200)
Message: XML Validation warning

As per twilio docs, updated the tag name for the supported TwiML tags.

Also, verified the same with the corresponding implementation in java sdk.
Ref: https://github.com/twilio/twilio-java/tree/main/src/main/java/com/twilio/twiml/voice
Ref Files:

  • SsmlBreak.java
  • SsmlEmphasis.java
  • SsmlLang.java
  • SsmlP.java
  • SsmlPhoneme.java
  • SsmlProsody.java
  • SsmlS.java
  • SsmlSayAs.java
  • SsmlSub.java
  • SsmlW.java

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

@AsabuHere
Copy link
Contributor

These files are auto generated in every release(if there are new changes). This fix need another PR to yoyodyne repo which generates these files. Otherwise these changes will be overwritten in the next release. The yoyodyne changes needs a detailed analysis and more work which may take some more time. We will keep the issue thread posted about the progress

@AsabuHere AsabuHere self-requested a review October 11, 2023 06:36
AsabuHere
AsabuHere requested changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@AsabuHere AsabuHere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes are not supposed to merge in this repo. These files are auto generated and not supposed to change. The changes have to be made in yoyodyne repo(internal to twilio), else these changes will be removed in next release

@abhishek-eltropy
Copy link
Author

@AsabuHere Ok. Thanks for the review and the explanation.

Since yoyodyne repo is internal to Twilio, I don't have access to it.
I will close this PR then. Hope, Twilio team will fix this sooner.

@AsabuHere
Copy link
Contributor

Sure, We are working on this issue in the current sprint. Can you share some code snippets with which this issue can be reproduced?
Thanks in Advance

@abhishek-eltropy
Copy link
Author 

ssmlBreak := twiml.VoiceBreak{
	Time: "1s",
}

chooseQueueSay := twiml.VoiceSay{
	Message:       "Please select one of the following",
	Voice:         "Polly.Joanna-Neural",
	InnerElements: []twiml.Element{ssmlBreak},
}

say1 := twiml.VoiceS{
	Words: "Press 1 for Loans.",
}
say2 := twiml.VoiceS{
	Words: "Pres 2 for queries.",
}

chooseQueueSay.InnerElements = append(chooseQueueSay.InnerElements, say1, ssmlBreak, say2)

voiceResponse, err := twiml.Voice([]twiml.Element{chooseQueueSay})
if err != nil {
	fmt.Println("Error: ", err)
	return
}

fmt.Println("voiceResponse", voiceResponse)

@abhishek-eltropy
Copy link
Author

abhishek-eltropy commented Oct 11, 2023 •
edited
Loading

Above snippet will generate following xml:

<?xml version="1.0" encoding="UTF-8"?> <Response> <Say voice="Polly.Joanna-Neural">Please select one of the following <Break time="1s"/> <S>Press 1 for Loans.</S> <Break time="1s"/> <S>Pres 2 for queries.</S> </Say> </Response>

However, it should be -
<?xml version="1.0" encoding="UTF-8"?> <Response> <Say voice="Polly.Joanna-Neural">Please select one of the following <break time="1s"/> <s>Press 1 for Loans.</s> <break time="1s"/> <s>Pres 2 for queries.</s> </Say> </Response>

Please note that break and s tags were capitalized in the generated xml.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AsabuHere AsabuHere AsabuHere requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@abhishek-eltropy @AsabuHere