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

[WIP] candy:core:chat:invite triggerHandler #4

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

malakada
Copy link

@malakada malakada commented Jan 7, 2015

This trigger isn't referenced anywhere else inside of Candy Core.

@@ -482,9 +482,19 @@ Candy.Core.Event = (function(self, Strophe, $) {
}

if(directInvite.length > 0) {
fromUser = Candy.Core.getRoster().get(msg.attr('from'));
Copy link
Member

Choose a reason for hiding this comment

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

So, open question (including @bklang): how do we treat users who aren't in our roster? Do we pass through the JID instead of a Contact, or do we pass something API-compatible with Contact but which nevertheless indicates absence of full details from the roster and is scoped only to this event?

Copy link
Member

Choose a reason for hiding this comment

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

I am inclined to think something API-compatible with a Contact, but with the missing fields returning nil/undefined or empty strings. May also add a boolean for "in roster?"

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
Author

Choose a reason for hiding this comment

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

Also 👍

@malakada malakada force-pushed the feature/triggerhandler_candy_core_chat_invite branch from b01d6f9 to 55ae884 Compare January 9, 2015 19:18
@@ -264,6 +264,7 @@ Candy.Core.Event = (function(self, Strophe, $) {
},

_addRosterItem: function(item) {
item.inRoster = true;
Copy link
Author

Choose a reason for hiding this comment

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

The item here comes directly from a callback on Candy.Core.getConnection().roster, so I'm making an assumption that if it's coming all the way to _addRosterItem() that it's for someone who can be safely said to be in our roster. It can get here either through it's sibling _addRosterItems() or through a direct callback created in Candy.Core.Action.Jabber.Roster().

Copy link
Member

Choose a reason for hiding this comment

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

👍

@malakada malakada force-pushed the feature/triggerhandler_candy_core_chat_invite branch from 55ae884 to 3d9ee4a Compare January 9, 2015 19:26
@@ -472,9 +481,11 @@ Candy.Core.Event = (function(self, Strophe, $) {
reason = reasonNode.text();
}

fromUser.jid = mediatedInvite.attr('from');
Copy link
Member

Choose a reason for hiding this comment

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

Why would we modify a contact's JID?

Copy link
Author

Choose a reason for hiding this comment

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

In this case, because it's empty. It's being set above as a blank new Candy.Core.Contact and that data isn't set. Because there are two paths here, --oh. Hi. I meant to remove this line when I gave up on trying to consolidate it into one place before the if's and just put them in separately. Thanks. :)

@malakada
Copy link
Author

malakada commented Jan 9, 2015

Our only plugin that uses candy:core:chat:invite is AutoJoinInvites. The change would be straightforward.

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

Successfully merging this pull request may close these issues.

3 participants