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.

Sign up for GitHub

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

Jump to bottom

Added Google OAuth Signup in the backend #112

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Added Google OAuth Signup in the backend #112

wants to merge 6 commits into from

Conversation

chinmaym07
Copy link
Member

Description

Fixes #30

Type of Change:

  • Code
  • Quality Assurance
  • Documentation

Code/Quality Assurance Only

  • This change requires a documentation update (software upgrade on readme file)
  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

osp
Screenshot_2021-03-05 Google Login – Django REST framework
Screenshot_2021-03-05 Google Login – Django REST framework(1)

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged
  • I have updated dependencies in requirements.txt

Code/Quality Assurance Only

  • My changes generate no new warnings

@codesankalp codesankalp added the Status: Needs Review PR needs an additional review or a maintainer's review. label Mar 5, 2021
@Amulya-coder
Copy link
Member

@chinmaym07 can you please fix the conflicts.

Amulya-coder
Amulya-coder suggested changes Mar 8, 2021
View reviewed changes
Copy link
Member

@Amulya-coder Amulya-coder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chinmaym07 could you please add some tests to this pr.

@codesankalp
Copy link
Member

@chinmaym07 Please resolve the merge conflicts and write a test for your implementation.

@devkapilbansal devkapilbansal added Status: Changes Requested Changes are required to be done by the PR author. and removed Status: Needs Review PR needs an additional review or a maintainer's review. labels Mar 16, 2021
@isabelcosta
Copy link
Member

@chinmaym07 can you please update your branch? That way tests may pass

@chinmaym07
Copy link
Member Author

@chinmaym07 can you please update your branch? That way tests may pass

Yes @isabelcosta I am writing a test for Google Oauth signup ..I'll update the branch with it .Sorry for the inconvenience

@isabelcosta
Copy link
Member

no inconvenience at all :)

@codesankalp
Copy link
Member

@chinmaym07 Any updates?

@chinmaym07
Copy link
Member Author

@codesankalp I will soon complete this ..Sorry for the delay..

@codesankalp
Copy link
Member

@chinmaym07 Can you update your progress @chinmaym07?

@codecov-commenter
Copy link

Codecov Report

Merging #112 (fb0f480) into develop (426773d) will increase coverage by 0.44%.
The diff coverage is 94.11%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #112      +/-   ##
===========================================
+ Coverage    64.88%   65.32%   +0.44%     
===========================================
  Files           64       65       +1     
  Lines         1031     1047      +16     
===========================================
+ Hits           669      684      +15     
- Misses         362      363       +1
Impacted Files Coverage Δ
main/settings.py 76.19% <83.33%> (+1.19%) ⬆️
token_auth/urls.py 100.00% <100.00%> (ø)
token_auth/views/google_oauth.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 426773d...fb0f480. Read the comment docs.

@chinmaym07
Copy link
Member Author

@codesankalp I have tried to write the test .
There is still the problem of mocking the api . I tried it .. but it didn't work out ..
Can you please have a look ..

codesankalp
codesankalp suggested changes May 11, 2021
View reviewed changes
tests/test_google_oauth.py
self._get_google_response()
user_count = get_user_model().objects.all().count()

response = self.client.post(self.google_signup_url, data=self.payload, format="json")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check the payload @chinmaym07. Why are you posting access_token in the payload?
The payload must contain the thing that we pass from the frontend?

token_auth/urls.py
@@ -14,4 +15,7 @@
# login URLs
path("token/", TokenObtainPairView.as_view(), name="token_obtain_pair"),
path("refresh/", TokenRefreshView.as_view(), name="token_refresh"),
# social-authentication
path("google/", GoogleLogin.as_view(), name="google_login"),
url(r"^accounts/", include("allauth.urls"), name="socialaccount_signup"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you are including all URLs of all-auth?

@codesankalp
Copy link
Member

@chinmaym07 Can you update your progress here?

@chinmaym07
Copy link
Member Author

@chinmaym07 Can you update your progress here?

So Sorry for the delay here .. Will update both the PR's by this week .

@isabelcosta
Copy link
Member

@chinmaym07 feel free to drop the issue. you can always come back later when you become available ;)

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

@Amulya-coder Amulya-coder Amulya-coder requested changes

@codesankalp codesankalp codesankalp requested changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Google OAuth register and login to OSP
6 participants
@chinmaym07 @Amulya-coder @codesankalp @isabelcosta @codecov-commenter @devkapilbansal