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

Slack API: share a learning to CodeBuddies directly from Slack (@cb_jarvis) #990

Closed
lpatmo opened this Issue Oct 2, 2018 · 18 comments

Comments

Projects
None yet
4 participants
@lpatmo
Copy link
Member

lpatmo commented Oct 2, 2018

Currently, users can schedule a hangout directly from Slack (see: #943).

On the site, we have a learnings tracker that hasn't gotten a lot of love (only ~300 learnings reported):

screen shot 2018-10-02 at 2 55 54 am

As you might be able to tell from the screenshot, these learnings are typically shared on codebuddies.org/hangouts, underneath a hangout iframe on a hangout page, or in a study group tab.

TODO:

  1. Let users share a learning directly from Slack by typing in @cb_jarvis #todayilearned [message]

  2. This message will appear as a learning under the Learnings collection, and be tied back to the user who submitted it.

screen shot 2018-10-02 at 2 59 20 am

  1. Take a look at #943 for guidance on how to align a CodeBuddies Slack user email with the same user email on codebuddies.org.

@lpatmo lpatmo changed the title Share a learning to CodeBuddies directly from Slack Slack API feature: share a learning to CodeBuddies directly from Slack Oct 6, 2018

@Bigocb

This comment has been minimized.

Copy link

Bigocb commented Oct 6, 2018

I would like to try this one for hacktober

@railsstudent

This comment has been minimized.

Copy link
Collaborator

railsstudent commented Oct 7, 2018

@Bigocb it's your to tackle

@railsstudent railsstudent added the claimed label Oct 7, 2018

@lpatmo lpatmo changed the title Slack API feature: share a learning to CodeBuddies directly from Slack Slack API: share a learning to CodeBuddies directly from Slack (@cb_jarvis) Oct 12, 2018

@lpatmo lpatmo removed the claimed label Nov 2, 2018

@railsstudent

This comment has been minimized.

Copy link
Collaborator

railsstudent commented Nov 2, 2018

Let me try to do it.

@lpatmo lpatmo added the high-priority label Nov 3, 2018

@lpatmo

This comment has been minimized.

Copy link
Member Author

lpatmo commented Nov 3, 2018

Thanks @railsstudent 🙏 For reference, this is Gaurav's PR: #943

Quick thought: maybe #til something I learned would be easier than @cb_jarvis #todayilearned [message]. I will ask on Slack to ask folks what syntax they prefer.

@railsstudent

This comment has been minimized.

Copy link
Collaborator

railsstudent commented Nov 3, 2018

@lpatmo Welcome, anything for Codebuddies. Let me study at #943 to see how the incredible @gauravchl did. I hope not to disturb him unless I am really stuck on the task.

@railsstudent railsstudent self-assigned this Nov 3, 2018

@lpatmo

This comment has been minimized.

Copy link
Member Author

lpatmo commented Nov 3, 2018

Looks like the syntax #til won the poll. :) So, no need to reference @cb_jarvis.

image

@railsstudent

This comment has been minimized.

Copy link
Collaborator

railsstudent commented Nov 14, 2018

@lpatmo @gauravchl Can I use the cb_jarvis bot to process todayILearnt event and then call meteor server to create today I learnt record in mongodb?

If not, I will need to create a new bot. Will each slack callback process by cb_jarvis and my new bot?

Please correct me if I am wrong. Should I add til logic to message-parser.js and webhook.js or make new message parser and webhook?

Please advise.

@gauravchl

This comment has been minimized.

Copy link
Collaborator

gauravchl commented Nov 14, 2018

@railsstudent Good question. I don't think it's good idea to reinvent the wheel. I'd vote to use the same cb_jarvis bot.

So there are two solutions:

  1. Instead using hashtag based commands like #til blah blah user can type @cb_jarvis til, blaah blaah. @lpatmo What do you think?

  2. Hashtag based command #til blah blah(without mentioning cb_jarvis) can be supported by jarvis as well. Will have to update some structure.
    For example updating this line:

    if (type !== "message" || !text || text.indexOf(`@${BOT_ID}`) < 0) return;
    const action = Parser.parse(text);

    with something like:

if(type !== "message" || !text) return;

const acceptedHashTags = ['#til'];
const jarvisMentioned = text.indexOf(`@${BOT_ID}`) > -1;
let  validHashtagMentioned = false;

acceptedHashTags.forEach(hashTag => {
if(text.trim().startWith(hashTag)) {
  validHashtagMentioned = true;
}
})

if(!jarvisMentioned && !validHashtagMentioned) return;

// Process further

Easier way is to implement the first one. Second one is also doable :)
Or support both, start with first one, see how it goes and then implement second one too.

@gauravchl

This comment has been minimized.

Copy link
Collaborator

gauravchl commented Nov 14, 2018

Should I add til logic to message-parser.js and webhook.js

Yup +1

@railsstudent

This comment has been minimized.

Copy link
Collaborator

railsstudent commented Nov 15, 2018

@railsstudent Good question. I don't think it's good idea to reinvent the wheel. I'd vote to use the same cb_jarvis bot.

So there are two solutions:

  1. Instead using hashtag based commands like #til blah blah user can type @cb_jarvis til, blaah blaah. @lpatmo What do you think?

  2. Hashtag based command #til blah blah(without mentioning cb_jarvis) can be supported by jarvis as well. Will have to update some structure.
    For example updating this line:

      [codebuddies/server/slack/webhooks.js](https://github.com/codebuddies/codebuddies/blob/52b44b3559ab45d0af15b02b3236c807229dba60/server/slack/webhooks.js#L53-L54)
    
    
        Lines 53 to 54
      in
      [52b44b3](/codebuddies/codebuddies/commit/52b44b3559ab45d0af15b02b3236c807229dba60)
    
    
    
    
    
        
          
           if (type !== "message" || !text || text.indexOf(`@${BOT_ID}`) < 0) return; 
        
    
        
          
           const action = Parser.parse(text); 
    

    with something like:

if(type !== "message" || !text) return;

const acceptedHashTags = ['#til'];
const jarvisMentioned = text.indexOf(`@${BOT_ID}`) > -1;
let  validHashtagMentioned = false;

acceptedHashTags.forEach(hashTag => {
if(text.trim().startWith(hashTag)) {
  validHashtagMentioned = true;
}
})

if(!jarvisMentioned && !validHashtagMentioned) return;

// Process further

Easier way is to implement the first one. Second one is also doable :)
Or support both, start with first one, see how it goes and then implement second one too.

@gauravchl I vote for the first one too and this is the reason I asked question 2. I did not want to tweak line 53 of message-parser.js worrying of breaking the logic that creates hangout from slack message.

cc @lpatmo

@railsstudent

This comment has been minimized.

Copy link
Collaborator

railsstudent commented Nov 15, 2018

@gauravchl If using the same cb_jarvis, what is the setup? Thanks.

@lpatmo

This comment has been minimized.

Copy link
Member Author

lpatmo commented Nov 15, 2018

@railsstudent

This comment has been minimized.

Copy link
Collaborator

railsstudent commented Nov 15, 2018

@lpatmo @gauravchl I will fly to Taipei to attend JSDC.tw conference this week but I can start working on it to set up the bot and formulate the solution next Monday. Thanks.

@lpatmo

This comment has been minimized.

Copy link
Member Author

lpatmo commented Nov 15, 2018

@gauravchl

This comment has been minimized.

Copy link
Collaborator

gauravchl commented Nov 15, 2018

Sweet!

If using the same cb_jarvis, what is the setup?

The setup is:

  1. Update the message-parser.js, add new command and reply inside ActionTable variable.

     const ActionsTable = [
       ...
       ...
     {
       command: "#til",
       reply: `Thank you! Your til message is being posted...`
     }
     ];
    

    reply is optional.

  2. Update webhook.js:

// Update processEvent function
if (action.command === "#til") {
  return webhooks.postTodayILearn(slackUserId, action, channel);
}



// create a new function postTodayILearn 
postTodayILearn(slackUserId, action, channel) {
   // Add a same logic as it is inside createHangout that checks
   // whether the slack user is also registered to codebuddies site 
   // may be refactor that logic from createHangout and make it reusable 

   // Next write a logic to post the til message to codebuddie's db
   // You can call the existing meteor method "addLearning" from `learnings/methods.js`
   // But keep in mind that it uses `this.userId`. 
   // And this.userId will always be undefined when calling that method from here.
   // You will have to refactor that method and pass the user who is creating the hangout explicitly.
   // For example see below commit where i refactored createHangout logic for the same purpose.
   // https://github.com/codebuddies/codebuddies/pull/943/commits/26729b0ec82a1d5c3632f23b86215908a0aea05c 

}

Let me know if you need any help.

Cheers!

@railsstudent

This comment has been minimized.

Copy link
Collaborator

railsstudent commented Nov 18, 2018

@gauravchl Thanks. I will make a stab at this issue after my trip.

@railsstudent

This comment has been minimized.

Copy link
Collaborator

railsstudent commented Dec 6, 2018

@gauravchl How can I obtain cbJarvisId in settings-development.json
Can I use any slack account to create an app, generate slack oauth token and replace the dummy value with the actual value in the json file?

Thanks.

@gauravchl

This comment has been minimized.

Copy link
Collaborator

gauravchl commented Dec 6, 2018

@railsstudent cbJarvisId is userid of the bot, you can hover/click on the bot name from slack web UI and it will show you the ID, ex:(BCKRMFPR6)

screen shot 2018-12-06 at 8 25 11 pm

Can I use any slack account to create an app, generate slack oauth token and replace the dummy value with the actual value in the json file?

Yes you can, I do also have a slack channel created for testing the jarvis, You can also test it there if you want. Let me know what's your email, will invite you there.

@lpatmo lpatmo closed this in #1083 Dec 12, 2018

@wafflebot wafflebot bot removed the in progress label Dec 12, 2018

@lpatmo lpatmo added the closed label Dec 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment