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

[New] SMS & Email Invite for Contacts #11

Merged
merged 12 commits into from
Dec 12, 2018
35 changes: 35 additions & 0 deletions packages/rocketchat-api/server/v1/misc.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,38 @@ RocketChat.API.v1.addRoute('directory', { authRequired: true }, {
});
},
});

RocketChat.API.v1.addRoute('invite.email', { authRequired: true }, {
post() {
if (!this.bodyParams.email) {
throw new Meteor.Error('error-email-param-not-provided', 'The required "email" param is required.');
}

Meteor.runAsUser(this.userId, () => Meteor.call('sendInvitationEmail', [this.bodyParams.email]));
return RocketChat.API.v1.success();

// sendInvitationEmail always returns an empty list
kb0304 marked this conversation as resolved.
Show resolved Hide resolved
/*
if(this.bodyParams.email in result){
return RocketChat.API.v1.success();
}else{
return RocketChat.API.v1.failure('Email Invite Failed');
}
*/
},
});

RocketChat.API.v1.addRoute('invite.sms', { authRequired: true }, {
post() {
if (!this.bodyParams.phone) {
throw new Meteor.Error('error-phone-param-not-provided', 'The required "phone" param is required.');
}
const phone = this.bodyParams.phone.replace(/-|\s/g, '');
const result = Meteor.runAsUser(this.userId, () => Meteor.call('sendInvitationSMS', [phone]));
if (result.indexOf(phone) >= 0) {
return RocketChat.API.v1.success();
} else {
return RocketChat.API.v1.failure('SMS Invite Failed');
}
},
});
1 change: 1 addition & 0 deletions packages/rocketchat-lib/package.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ Package.onUse(function(api) {
api.addFiles('server/methods/robotMethods.js', 'server');
api.addFiles('server/methods/saveSetting.js', 'server');
api.addFiles('server/methods/sendInvitationEmail.js', 'server');
api.addFiles('server/methods/sendInvitationSMS.js', 'server');
api.addFiles('server/methods/sendMessage.js', 'server');
api.addFiles('server/methods/sendSMTPTestEmail.js', 'server');
api.addFiles('server/methods/setAdminStatus.js', 'server');
Expand Down
64 changes: 64 additions & 0 deletions packages/rocketchat-lib/server/methods/sendInvitationSMS.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { Meteor } from 'meteor/meteor';
import { TAPi18n } from 'meteor/tap:i18n';
import { check } from 'meteor/check';
import _ from 'underscore';

Meteor.methods({
sendInvitationSMS(phones) {
const twilioService = RocketChat.SMS.getService('twilio');
Copy link

Choose a reason for hiding this comment

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

This hard codes twilio as the only sms service? Are there other services that admins might want to use? Is it possible to generalize this and pass the desired service into the method somehow?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Depending on what I saw, right now in RC there is only option for Twilio as the supported SMS service. They have made it partially modular, where anyone if wants to implement another service can implement it, but currently only Twilio is implemented. If we want to support multiple services, I can add a setting via which you can select which provider to use for inviting purpose, but that is only usable when other SMS services are implemented.

Copy link

Choose a reason for hiding this comment

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

So upstream has the ability to configure another SMS service, but no way to use it? Is that what you mean by "partially modular"?

Is it a lot of work to "add a setting via which you can select which provider to use for inviting purpose"?

Copy link
Collaborator Author

@kb0304 kb0304 Dec 10, 2018

Choose a reason for hiding this comment

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

https://github.com/RocketChat/Rocket.Chat/tree/develop/packages/rocketchat-sms/server
Here, only the Twilio is implemented. To use another, one will have to make service and register.
We can create an option to select from the available services (not a lot of work). Should we use Twilio as the default? There is nothing like default/active service for SMS.

Copy link

Choose a reason for hiding this comment

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

If it's not a lot of work, I think it's best. Do you agree? Just create the option, and use Twilio as the default. That way it is at least capable of using another service if an admin wants to make another service and register it. Thanks!

Copy link

Choose a reason for hiding this comment

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

I'm thinking that we might want to merge this PR and make notes on work like this that we'd like to do as enhancements. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, let's do it in another PR.

if (!RocketChat.SMS.enabled || !twilioService) {
throw new Meteor.Error('error-twilio-not-active', 'Twilio service not active', {
method: 'sendInvitationSMS',
});
}

const messageFrom = RocketChat.settings.get('Invitation_SMS_Twilio_From');
if (!twilioService.accountSid || ! twilioService.authToken || !messageFrom) {
throw new Meteor.Error('error-twilio-not-configured', 'Twilio service not configured', {
method: 'sendInvitationSMS',
});
}

check(phones, [String]);
if (!Meteor.userId()) {
throw new Meteor.Error('error-invalid-user', 'Invalid user', {
method: 'sendInvitationSMS',
});
}

// to be replaced by a seperate permission specific to SMS later
if (!RocketChat.authz.hasPermission(Meteor.userId(), 'bulk-register-user')) {
throw new Meteor.Error('error-not-allowed', 'Not allowed', {
method: 'sendInvitationSMS',
});
}
const phonePattern = /^\+?[1-9]\d{1,14}$/;
const validPhones = _.compact(_.map(phones, function(phone) {
if (phonePattern.test(phone)) {
return phone;
}
}));
const user = Meteor.user();
let body;
if (RocketChat.settings.get('Invitation_SMS_Customized')) {
body = RocketChat.settings.get('Invitation_SMS_Customized_Body');
} else {
const lng = user.language || RocketChat.settings.get('language') || 'en';
Copy link

Choose a reason for hiding this comment

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

Can you clarify this for me please?

Is "user.language" in this case the user who is sending the invitation? If so, is there some condition when we won't be getting that flag from the user, so that we need these '||' statements?

Is RocketChat.settings.get('language') the language that the admin uses on the RC server? (Which we know will be English because we are going to be the admins)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I replicated it from the RC code here. https://github.com/RocketChat/Rocket.Chat/blob/14f47007aec90aee4ed810d1250bc71f328df7f0/packages/rocketchat-livechat/messageTypes.js#L37

I will checkout for the cases (if they exist) when the first two will not be set.

Copy link

Choose a reason for hiding this comment

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

Interesting..... the file you linked to is "live chat", and the user will be on a web browser I believe.... so we can likely expect that we'll get "user.language", because web browsers do that by default. We should verify what this looks like when it's coming from the android client, because we most definitely want to send the invite in the native language.

Copy link

Choose a reason for hiding this comment

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

Make a note of this and track for future PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good.

body = TAPi18n.__('Invitation_SMS_Default_Body', {
lng,
});
}
body = RocketChat.placeholders.replace(body);
validPhones.forEach((phone) => {
try {
twilioService.send(messageFrom, phone, body);
} catch ({ message }) {
throw new Meteor.Error('error-sms-send-failed', `Error trying to send SMS: ${ message }`, {
method: 'sendInvitationSMS',
message,
});
}
});
return validPhones;
},
});
2 changes: 1 addition & 1 deletion packages/rocketchat-sms/server/services/twilio.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class Twilio {
send(fromNumber, toNumber, message) {
const client = twilio(this.accountSid, this.authToken);

client.messages.create({
return client.messages.create({
to: toNumber,
from: fromNumber,
body: message,
Expand Down
26 changes: 26 additions & 0 deletions packages/rocketchat-sms/server/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,31 @@ Meteor.startup(function() {
i18nLabel: 'Auth_Token',
});
});

this.section('Invitation', function() {
Copy link

Choose a reason for hiding this comment

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

This PR does not include anything related to the deep link to a DM with the original inviter, in the invitation message, is that correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct.

Copy link

Choose a reason for hiding this comment

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

How about any default SMS message at all? Should that be implemented here, or will that come in a different PR? As it is, only "Invitation_SMS_Default_Body" shows up in the sms invite message.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lets do that in another PR where we handle the Native language case as well.

this.add('Invitation_SMS_Twilio_From', '', {
type: 'string',
i18nLabel: 'Invitation_SMS_Twilio_From',
});
this.add('Invitation_SMS_Customized', false, {
type: 'boolean',
i18nLabel: 'Custom_SMS',
});
return this.add('Invitation_SMS_Customized_Body', '', {
type: 'code',
code: 'text',
multiline: true,
i18nLabel: 'Body',
i18nDescription: 'Invitation_SMS_Customized_Body',
enableQuery: {
_id: 'Invitation_SMS_Customized',
value: true,
},
i18nDefaultQuery: {
_id: 'Invitation_SMS_Default_Body',
value: false,
},
});
});
});
});