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.

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

No longer sets language again during cf_callflow or pivot cf exec #1264

Closed

Conversation

danielfinke
Copy link
Member

#1252 against master

whapps_call:set_language(wh_util:to_lower_binary(Language), Call);
{'error', _E} ->
lager:debug("no source endpoint for this call, setting language to default ~s", [Default]),
whapps_call:set_language(Default, Call)
Copy link
Contributor

Choose a reason for hiding this comment

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

@danielfinke
I don't think I understand this case clause. If the default language is already set on the call and there is no source endpoint set the language of the call to the default (wouldn't the case clause ensure that already)?

I suspect the intent is to only set the language if the language has not already been set but that doesn't seem like the safest means to make this determination. It basically means if the language is set to something other than default it can not be changed during re-hunt, but if it is set to default (or just default) then it could be. No?

Could you describe your use-case maybe it will clear things up or let me come up with some suggestions :)

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@danielfinke
Ah ok I understand now, thanks :)

SO I would prefer Luis's comment on the other PR:

"yes, i understand the path now. thank you.
maybe we should change how resume works because it still execs the other actions in bootstrap.
let me run some tests."

@lazedo you left it at "let me run some tests", let us know what you find :)

My other thought would be to ensure the language is not set unless explicitly provided and all functions that access this information fallback to the default if not present (perhaps even in whapps_call). This way 'undefined' means 'not set use default', and any other value means 'this is what the user wants'. If we did this then I would feel much better about ignoring the language if set in route_win.

@jamesaimonetti
Copy link
Member

@danielfinke where are we with this PR? Can you address @k-anderson's concerns in the last comment from him? Thanks

@jamesaimonetti
Copy link
Member

@danielfinke We're going to close this for now so we can start to get our PR queue down to those PRs being actively worked on. Please open a new PR (and reference this one!) when you have the time to address the last comments above from @k-anderson.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants