-
-
Notifications
You must be signed in to change notification settings - Fork 102
Develop #28
base: develop
Are you sure you want to change the base?
Develop #28
Conversation
@hari-n thanks for providing this. I'm not quite sure what's going on with the changes you made outside of
|
…r instead all the fields in the dict
…anged it to connection.user_id
Here it is. Erik as you suggested, I split the changes into separate commits for you to cherrypick. Hope this helps. |
Unlike the test_app, I have implemented Connection differently using a user_id ReferenceField to User. So please discard that commit. |
@hari-n the tests pass locally with the single linkedin file added (and Travis should represent that here soon, knock on wood). Note that the tests to actually touch these provider files, so the fact that the tests pass doesn't say that much. With respect to the other changes, it's generally the best idea to implement on your end the way that it's done in the tests. Think of the tests as extended documentation (this is in general with open source, not just for this project). With your implementation you may run into other headaches down the road, and we won't be able to help or merge in PR for bug fixes that arise from your revised implementation. I opened issue #29 to figure out where this update should go. Also, with respect to the |
@eriktaubeneck the 'linkedin.LinkedInAuthentication' object is capable of doing oAuth authentication with linkedin by itself. But, when the 'get_provider_user_id' and 'get_connection_values' methods are called, flask-social has already done the oAuth. So, I just passed the token to 'LinkedInAuthentication'. In get_api, previously stored 'connection' object is passed to the method. If the 'access token' in the connection is expired, then 'LinkedInAuthentication' can acquire new token using the consumer_key and the consumer_secret. Also, the person using the api might need permission from the user for a wider scope. Of the 4 commits, I agree the last 2 commits (bottom two i.e. changes to 'views.py') are my implementation specific. That said, I would add, instead of using all the values in dict to query for a 'connection', using just the key values would be better (3rd commit from the top). Also, do you have a test case where the Flask app is run on a sub directory like http://localhost/myapp? If so, you will see the URL redirect issue (2nd commit from the top). Thanks lot of the guidance on using tests as an extended documentation. It would have stopped me from getting some of the headaches I had, but I also learnt a lot in during the debug process :-) |
WRT the subdirectory, what do you have set as the |
@hoprocker please see my last response to eriktaubeneck. I had that issue because of the way I referenced the User class within the Connection class. Once I saw how User class is referenced in the test_app/mongoengine.py, I corrected my implementation. So please discard that pull request. |
Gotcha, and thanks for taking a stab at this implementation. |
Thanks for the social extension to Flask. I am using it to authenticate against linkedin. So, here is a my small contribution to your effort.
BTW, this is my first development in opensource. So please review thoroughly. Thx