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

Nice UI for adding a contact through a Gateway #378

Merged
merged 8 commits into from Apr 19, 2017

Conversation

singpolyma
Copy link
Contributor

@singpolyma singpolyma commented Apr 2, 2017

Implements https://xmpp.org/extensions/xep-0100.html#addressing as well as a final fallback to the old %-encoding hack that some gateways still use for historical reasons.

Closes #367

Depends on https://github.com/movim/moxl/pull/17

@singpolyma singpolyma force-pushed the jabber-iq-gateway branch 4 times, most recently from e9b25fe to f45ade8 Compare April 2, 2017 22:53
If there are any.  Same as the old UI when there are no gateways.
@singpolyma singpolyma changed the title [WIP] jabber:iq:gateway Nice UI for adding a contact through a Gateway Apr 3, 2017
@singpolyma
Copy link
Contributor Author

@edhelas this one is ready for initial review now, and is fully functional in my tests :)

Copy link
Member

@edhelas edhelas left a comment

Choose a reason for hiding this comment

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

Looks great :)

$this->rpc(
'Roster.addGatewayPrompt',
$packet->from,
''.$packet->content->prompt,
Copy link
Member

Choose a reason for hiding this comment

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

When you're casting to string please use (string)$variable and not ''. ;)


function onIqGatewaySet($packet)
{
$form = json_decode($packet->content->extra);
Copy link
Member

Choose a reason for hiding this comment

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

No need of this extra step if it's an array, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was getting an error about not serializing SimpleXML without this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

after other changes, this no longer happens, it seems. So I've removed the json encoding again

If the Gateway doesn't actually support jabber:iq:gateway, use a
reasonable fallback.
Otherwise we sometimes get errors about not being allowed to serialize
SimpleXML stuff.
If we got a prompt back earlier, then we know that the gateway supports
jabber:iq:gateway.  So we send off the user input, and then re-run our
form handler on whatever comes back.

Don't dismiss the dialog right away in JavaScript, but wait for the
server to be really done first.
The gateway may perform some validation on the user input, and we need
to show any errors to the user so they can correct their mistakes.
@singpolyma
Copy link
Contributor Author

@edhelas I believe I have made all your suggested changes :)

@edhelas edhelas merged commit a355516 into movim:master Apr 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants