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

Autojoin channels based on signup flow information #47

Open
hollomancer opened this issue Jun 22, 2019 · 14 comments
Open

Autojoin channels based on signup flow information #47

hollomancer opened this issue Jun 22, 2019 · 14 comments
Labels
enhancement New feature or request

Comments

@hollomancer
Copy link
Member

As a user, I want to autojoin a Slack channel corresponding to my military status (milspouse / active duty), so that I have a new user experience tailored to my current life situation.

Product Requirements

  • (MVP) Users on signup that identify as a military spouse autojoin #milspouses.
  • Users on signup that identify as active duty autojoin #active-duty.
@hollomancer hollomancer added the enhancement New feature or request label Jun 22, 2019
@brownnrl
Copy link

brownnrl commented Sep 27, 2019

I'm new here, but this looks liked a combined feature between the backend passing the e-mail plus the military status. Either the backend or the pybot will need to keep an association between the status and the channel.

It looks like the pybot is using the undocumented user invitation referred to in this SO post. So if the backend could pass the e-mail and an optional status, then the pybot could take that and associate that with the correct channel id's to include on the invitation.

@brownnrl
Copy link

So initially, when the slack invitation is created the user profile may not have the military status information.

The user can update their profile at any time to change their current status with incremental updates to their profile. They can also change their status before signing onto slack via their e-mail link, or not update their profile and log onto slack.

So check would be performed after both a slack ID is given from the bot to the back-end or the front end communicates a status update to their profile. If the check was successful that both slack id and military status exist in the DB, then a call the python bot would be made invite the user to the associated channel.

For after invitation joins, a python bot would invite a user to the channel via the slack API channels.invite. However, the python bot would need to be a member of that channel according to the documentation. It would need to sit on activeduty and milspouses.

It looks like milspouses is a shared channel, so I don't know if adding the bot to the channel would be permitted?

@mdaizovi
Copy link

mdaizovi commented Oct 2, 2019

Sounds fun. If it can wait until the weekend I'd like to have a crack at it.

brownnrl added a commit to brownnrl/back-end that referenced this issue Oct 2, 2019
…p info flow

This adds a new background task to communicate to pybot.

When an update is performed, it checks that the slack id and military
status is present and not equal to old values and kicks off the
background task.

The test is not yet complete, it needs to exercise the background task.
@brownnrl
Copy link

brownnrl commented Oct 2, 2019

@mdaizovi I committed the WIP I have so far and on OperationCode/operationcode-pybot#110 on my fork and my pybot fork. I was just going to put some better unit/integration tests on it tonight.

It's not really complete. In the pybot I don't do channel invitations just extended the the existing messages when slack-id and status is present and changes. I haven't tested it actually working with Slack in the loop.

I'll be out camping this weekend and won't be able to work on it after tonight. If you want to take it from there this weekend and review/clean up/scrap and rework/change/test and put a PR together, I'm cool with that if you want to team up or take it over yourself.

brownnrl added a commit to brownnrl/back-end that referenced this issue Oct 3, 2019
Tests that when an update is made to mil status or slack id the status is
pushed to pybot.

Doesn't push to pybot when those fields don't change or when only one
is present.
@brownnrl
Copy link

brownnrl commented Oct 3, 2019

Ok, I did the test last night and pushed that up just not in the commit above so it looks like it's working in my local environment. I won't be able to work on this anymore until later next week.

@mdaizovi did you want to take it over from here with the pybot changes too? I didn't get a chance to write any tests for pybot. You can take credit for both if you want. If not, I'll pick it up again later next week. :)

@mdaizovi
Copy link

mdaizovi commented Oct 7, 2019

Hey @brownnrl , I'm very happy to collaborate with you on this but it looks like you've done most of the work! I apologize for my non-response, hard to find time after work and familiarizing myself w/ codebase. I do have a few things 'd like to add, so I'd love to do them this week, but if you get to it first, no problem, there will be other issues.

@brownnrl
Copy link

brownnrl commented Oct 8, 2019

No worries. I'd actually prefer to collaborate with you. After work time flies with family stuff, but I would like to contribute, help others contribute, and learn from how other people tackle issues.

I think that the ticket on this side is fairly complete, but the pybot changes are still incomplete on the other side. I have had the front end and back end communicating, but I don't have experience simulating the pybot locally (I haven't tried). As I said before, I don't have any unit / automated integration tests implemented for the pybot changes.

If you have any roadblocks I'll be happy to take a second look too. We can also connect on the opcode slack as well.

@mdaizovi
Copy link

mdaizovi commented Oct 8, 2019

Great! I'm not finished, but this is what I'm thinking: the user should be auto joined, so some of the status data should be included in the initial invite, and then also sent as a message if the status changes. Also, we should try to abstract a little so that if more status channels are added it's easy to add this functionality. I haven't finished yet, nor have I tested it, and I think the dicts could be put in a better place, but this is where I'm thinking we should go with it. Your thoughts?

@brownnrl
Copy link

brownnrl commented Oct 8, 2019

On your first point, the problem I discovered with including it in the initial slack invite is that the military status information might not be present when the slack invite is sent out.

Currently, a new user signs up with email / password / name / zip code. Then the back end at that point sends the initial slack invite and email confirmation. Then the new user can login, join the slack, confirm their e-mail, and can update their profile with status. All of those things can be done independently. So they could update their profile first and not have a slack id, or join slack but not have annotated their status. So if you wanted it to be on initial invite, you'd have to change the front end as well perhaps by including military status in the initial form. But for a first pass, it probably isn't necessary.

If we use the channels.invite to use the pybot to invite them to the channel in place of where you have your commit then whenever they get around to both updating their military status and joining the slack the new user would maybe get a special message and an invite.

On your second point, an abstraction of all the functions that should be performed when a new user joins is a good idea.

However, all that needs a few tests in the pybot code to confirm those abstractions work as intended before we do any pull requests. It would also be nice to try it out with slack in the loop before a PR, but I don't have much experience in that area. Maybe I'll ping the slack channel later in oc-python-projects to see how they normally test new pybot features. I saw a pybot channel for that purpose, so I'm guessing that is how.

@mdaizovi
Copy link

mdaizovi commented Oct 9, 2019

I see. Well I bet we could get the background task to wait about a minute before sending the invite, and then we could check status. I bet 99% of people keep going, filling out their status, and never changing it, and also don't care if their email comes immediately after signup or a minute after signup, so it would be cool it we could just wait a minute to catch them. Once again I have to wait until weekend to implement this, but I bet we can ge something smooth and elegant going here.

@brownnrl
Copy link

Cool. Yeah, delaying the task would probably catch most. Using updates would catch the rest.

Let me know if you want any help with implementation or tests after this weekend. Likely won’t be able to touch it this weekend but will have time Monday.

@brownnrl
Copy link

@mdaizovi Hi mic, did you get a chance to work on this over the weekend? Let me know if you want any help.

@mdaizovi
Copy link

mdaizovi commented Oct 20, 2019

Yeah kinda. I really want to watch the signup flow go but each of the readmes only describe that repo, I haven't seen one that describes getting a full working local dev env set up. I forked and cloned all 3 of the main repos and kind of had one going with just a local virtual environment, but when I tried to run front end and back end and create a user as if in production, I'm getting a lot of errors (sentry, KeyError: 'SLACK_TOKEN', etc...) I bet I wouldn't get if I set up docker or whatever it is I'm properly supposed to do. Do you know if this document exists, or do I need to just figure it out?

@brownnrl
Copy link

It looks like they have a test slack. OperationCode/operationcode-pybot#69

We can get a test token either via that or create another slack test channel. Probably best to get a recommendation from someone that owns the process on oc-python-projects channel.

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

No branches or pull requests

5 participants