Request expert feature

[paulocheque]

This PR is not complete yet. I am still working on it.

[durden]

Looks like a good start, just a few comments:

1. What does upgrading the oauthlib get us?
   • It's no problem to upgrade. I'm just curious what the benefits are so we can have it documented here.
2. I think we should keep the hack hands stuff separated from the main views and remote.py.
   • The reasoning here is the hack hands stuff will most likely be used by very few users. We can make it slightly easier to navigate the code by separating this off too. This way potential contributors don't have to worry about this integration at all unless they specifically need it.
   • Maybe we should make a remote sub package and put in a github.py and a hackhands.py file?
   • The easier option is to put all the hack hands views in hackhands_views.py and the remote oauth stuff in hackhands.py.
     • These are just examples so let me know if you have preferences on the naming or layout.

[paulocheque]

Hi,

1. I was stucked with some strange erros during the OAuth requests to the hack.hands API, so I decide to check the Flask-oauthlib for issues.
   In fact, the errors I was facing was not related to the lib, but since this new version had some fixes (https://github.com/lepture/flask-oauthlib/blob/master/CHANGES.rst), I updated to advance my research.
2. I moved to a separated module (hackhands.py). But there are some lines that need to be in the core of the system: like some session variables.

[paulocheque]

In the logout view, why dont we just call session.clear()?

[durden]

Good point. There's no reason to not call clear. Only downside is clearing
the previously viewed page if we need a redirect but even that isn't very
useful.

On Wednesday, May 18, 2016, Paulo Cheque notifications@github.com wrote:

In the logout view, why dont we just call session.clear()?

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#77 (comment)

[durden]

Sounds good. Upgrading isn't a problem I was just curious if there was a
critical doc we needed to make note of. Thanks for separating out the
hackhands stuff. It shouldn't be a problem if the hackhands integration
puts something in the main app config or the main app context in
init.py, etc.

On Wednesday, May 18, 2016, Paulo Cheque notifications@github.com wrote:

In the logout view, why dont we just call session.clear()?

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#77 (comment)

[paulocheque]

I guess the feature has been done. You can check on the dev env.

Important Considerations

New env vars:

To deploy in a new environment (including the production), we need to define the new env vars on Heroku:

heroku config:set HACK_HANDS_BASE_URL="..." --app ...
heroku config:set HACK_HANDS_CLIENT_ID="..." --app ...
heroku config:set HACK_HANDS_CLIENT_SECRET="..." --app ...

Authorization / Config

We only configured the staging env for a while. Before deploying for production, we need to configure hack.hands() for the new env.

Storage

I am using Redis to store this information. I setted the timeout to None so this information will never expire. But we need to consider the Redis as a true storage from now on. I mean, we cannot clear the cache of this new information.

[durden]

We should use url_for here so we don't hard-code the domain, etc.

[durden]

I looked through this and looks good. I've added 1 comment to the diff:

Using url_for

[durden]

I think we should hold off on this feature until the new design is pushed to production and fully tested. I don't think this feature will change once the new design is in place, but it's easier to integrate one big change at a time. Also, we can rebase this feature after the new design changes are in and re-test everything.

[paulocheque]

Nice catch! Fixed.

[durden]

Is your test site running this version? If so, how do I attempt to hook up the hack hands feature?

[durden]

We need to consider the workflow for this once we roll it into production. Maybe we could put a link at the top of the profile page to link a hackhands account? This way each time a user logs in they see the link, but it's not intrusive. Of course, the link should only show up if the user is viewing their own profile page.

[paulocheque]

I deployed into the test site this feature. Right now, I added a link into the navbar. I can move the link to another place after feedbacks.

[durden]

@paulocheque Thanks for deploying it for testing! Just for historical reference here's what the feature looks like now:

I vote we move the 'connect with hackhands link' to the dropdown menu under the users' avatar when they are logged in. So that menu will then have 'my guides', 'connect to hackhands', and 'logout'.

We should also add a link to 'connect to hackhands' on the profile page in the following case:

• User is viewing their own profile
• User is logged in
• User doesn't already have the account linked

We could add this link at the top of the profile 'body' and use something like the CSS class bg-info to make the link show up as a 'banner' in a light blue color. This is just a suggestion though so feel free to try some other styling if you have other ideas.

[paulocheque]

Cool! Added all changes. I deployed in the test site:

1
2

The button colors are messy because I deployed both PRs in the same branch. But after the Design PR being accepted, the color should be fixed.

[durden]

Looks great. Let's finish up the design changes, rebase this branch, and then double-check the button colors. Thanks!

[paulocheque]

Rebased and fixed the button layout.

[paulocheque]

Rebased to origin/master