-
Notifications
You must be signed in to change notification settings - Fork 3
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
added quick reply feature #3
Conversation
Please note that some changes are required in the botmaster repository to implement quick reply feature in botmaster-twitter-dm. |
Thanks for this request. I've added a few comments above so that I can merge the code in. Should be like 5 minutes to do so. Cheers |
This code is sufficient to enable quick reply feature for botmaster-twitter-dm module.
lib/twitter_dm_bot.js
Outdated
__sendMessage(rawMessage, send_options, quick_reply = false) { | ||
console.log("rM"); | ||
console.log(util.inspect(rawMessage, false, null)) | ||
|
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.
These console.logs should be removed
lib/twitter_dm_bot.js
Outdated
if (message.quick_replies) { | ||
// construct the quick replies for twitter platform | ||
|
||
let newObj = { |
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.
Rename newObj
to quickReplyFormattedMessage
and should be const
lib/twitter_dm_bot.js
Outdated
resolve(data); | ||
}); | ||
}); | ||
} |
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 code should look like this:
return new Promise((resolve, reject) => {
// if sending events like quick replies, add /events to url
this.twit.post(`direct_messages${rawMessage.event ? '/events/' : '/'}new`, rawMessage, (err, data) => {
if (err) {
reject(err);
}
resolve(data);
});
});
or if wanting to be more explicit:
// if sending events like quick replies, add /events to url
const sendMessageUrl = rawMessage.event ? 'direct_messages/events/new' : 'direct_messages/new'
return new Promise((resolve, reject) => {
this.twit.post(sendMessageUrl, rawMessage, (err, data) => {
if (err) {
reject(err);
}
resolve(data);
});
});
lib/twitter_dm_bot.js
Outdated
|
||
// convert default quick reply structure to twitter supported structure | ||
|
||
let data = message.message.quick_replies; |
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.
data
should be renamed to quickReplies
. It should be a const
as the object is never overwritten. You should be using the eslint config from the project here (found in the lib directory)
lib/twitter_dm_bot.js
Outdated
metadata: data[d]["title"] | ||
} | ||
quick_replies.push(temp) | ||
} |
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 whole code should be changed to:
const quickReplies = message.message.quick_replies.map(quickReply => ({
label: quickReply['title'],
metadata: quickReply['title']
}))
If you would rather use a loop, it should look like this:
const quickReplies = []
for (const quickReply of message.message.quick_replies) {
quickReplies.push({
label: quickReply['title'],
metadata: quickReply['title']
})
}
lib/twitter_dm_bot.js
Outdated
text: message.message.text, | ||
quick_reply: { | ||
type: "options", | ||
options: quick_replies |
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 should then use the quickReplies
object
lib/twitter_dm_bot.js
Outdated
} | ||
quick_replies.push(temp) | ||
} | ||
let quickReplyObject = { |
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 should be a const
as it is not reassigned
Hi @jdwuarin, |
@abhishekchotaliya - looking good to me. If you can confirm that this all works for you with both quick replies and normal messages, I'll go ahead and merge it. |
Yeah, it is working with both normal messages and quick replies. |
No description provided.