-
Notifications
You must be signed in to change notification settings - Fork 615
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
Fixes join when password is set for room. #578
Conversation
_join_room() was expecting compat_str() to bail out with None. That is not the case anymore (commit 4ed92ff). Therefore room JID was junk and password enabled XMPP MUC rooms could not be joined.
room_name = compat_str(room) | ||
if room_name is not None: | ||
room, username, password = (room_name, self.bot_config.CHATROOM_FN, None) | ||
if isinstance(room, tuple): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking for just tuples won't be sufficient here as a user could be using lists as well, which would give the same behaviour.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok. I saw only tuples in config template so that's what I thought can ever exist:
https://github.com/errbotio/errbot/blob/master/errbot/config-template.py#L149
Damn, I thought I'd checked all the callers of that function so carefully 😞 Thanks for tracking down the issue and suggesting a fix. Just have a minor comment on it. |
room_name = compat_str(room) | ||
if room_name is not None: | ||
room, username, password = (room_name, self.bot_config.CHATROOM_FN, None) | ||
if isinstance(room, tuple) or isinstance(room, list): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be written as if isinstance(room, (list, tuple)):
@mslehto I'm going to merge this when I have time tomorrow (go ahead and poke me again if I fail to). I left a minor comment but don't worry about it, I'll change it myself when I merge it so you don't have to bother with such a trivial change yourself. |
Fixes join when password is set for room.
See fd23ab7 😄 Thanks! |
_join_room() was expecting compat_str() to bail out with None.
That is not the case anymore (commit 4ed92ff).
Therefore room JID was junk and password enabled XMPP MUC rooms could not be joined.