Skip to content

Feature - Github OAuth Flow#22

Open
huzefa-amura wants to merge 6 commits intosell-do:developfrom
huzefa-amura:feature-github-oauth
Open

Feature - Github OAuth Flow#22
huzefa-amura wants to merge 6 commits intosell-do:developfrom
huzefa-amura:feature-github-oauth

Conversation

@huzefa-amura
Copy link
Copy Markdown
Contributor

No description provided.

Comment thread src/actions/index.js
@@ -0,0 +1,14 @@
import { SIGN_IN, SIGN_OUT } from './types';

export const signIn = (accessToken) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If these actions are specific to GithubAuth compoent then kindly keep these actions in GithubAuth actions file

import { signIn, signOut } from '../actions';

const CLIENT_ID = "f62e1f4242f2975a640f";
const REDIRECT_URI = "http://localhost:3000/";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

make URLs env specific


fetch(`http://localhost:5000/authenticate/${code}`)
.then(response => response.json())
.then(({ token }) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

kindly handle fail case

onSignOut = () => {
localStorage.setItem("authDetails", JSON.stringify({}));
this.props.signOut();
window.location.reload();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Instead of reloading cant we manipulate state of parent comonent or somthing like that. Reloading page kills the purpose of SPA

switch (action.type) {
case SIGN_IN:
return { ...state, isSignedIn: true, accessToken: action.payload };
case SIGN_OUT:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If we are setting everything to INITIAL_STATE, can't we just return INITIAL_STATE

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants