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 #112

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

Added Google Oauth Signup #112

wants to merge 6 commits into from

Conversation

chinmaym07
Copy link
Member

@chinmaym07 chinmaym07 commented Mar 5, 2021 •
edited
Loading

Description

Fixes #111

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?

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

Code/Quality Assurance Only

  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes

For the frontend part of this issue . I took some help from @codesankalp's PR for github auth.

osp

@codesankalp codesankalp added the Status: Needs Review PR needs an additional review or a maintainer's review. label Mar 5, 2021
codesankalp
codesankalp suggested changes Mar 13, 2021
View reviewed changes
Copy link
Member

@codesankalp codesankalp left a comment

Choose a reason for hiding this comment

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

@chinmaym07 Please add a test for this.

@codesankalp codesankalp 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 13, 2021
@codesankalp
Copy link
Member

@chinmaym07 Please also resolve the merge conflicts after writing test for this.

@codesankalp
Copy link
Member

@chinmaym07 Any updates?

@chinmaym07
Copy link
Member Author

chinmaym07 commented May 9, 2021 •
edited
Loading

@codesankalp Sorry for the delay.. I was not well for back few days & this pr just slipped from my mind ..
I have added the test & ran it locally .. It is working & resolved the conflicts & ran the npm run lint:fix to run the linters ,
But still the test is failing .. Can you please help me with this ....

Screenshot from 2021-05-09 22-39-07

Peek.2021-05-09.22-36.mp4

codesankalp
codesankalp reviewed May 9, 2021
View reviewed changes
Copy link
Member

@codesankalp codesankalp left a comment

Choose a reason for hiding this comment

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

The code LGTM. Will test this PR locally tomorrow.

docs/docusaurus.config.js
@@ -43,8 +43,7 @@ module.exports = {
items: [
{
label: 'Backend Docs',
href:
'https://github.com/anitab-org/anitab-forms-backend/wiki',
href: 'https://github.com/anitab-org/anitab-forms-backend/wiki',
Copy link
Member

Choose a reason for hiding this comment

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

Don't make changes unrelated to this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just ran npm run format. It's only formatting change.. Should I remove it ?

Copy link
Member

Choose a reason for hiding this comment

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

Please update your dependency and reformat the code.

.env.sample
@@ -0,0 +1,2 @@
REACT_APP_GOOGLE_CLIENT_ID=<Google App Client ID>
REACT_APP_GOOGLE_CALLBACK_URL=<Google Callback URL>
Copy link
Member

Choose a reason for hiding this comment

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

Add a new line at the end.

@codesankalp
Copy link
Member

@chinmaym07 Can you do the suggested changes?

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

@codesankalp codesankalp codesankalp requested changes

At least 2 approving reviews are 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 SignUp
2 participants
@chinmaym07 @codesankalp