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.

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

Implement authentication and user modules #9

Open
wants to merge 123 commits into
base: master
Choose a base branch
from

Conversation

Sasha-belokur
Copy link

@Sasha-belokur Sasha-belokur commented May 10, 2019

This pull request will add authorization and user modules. Both frontend and backend are implemented using TypeScript. All authorization flows (JWT, session, socials) implemented using passport.

Copy link
Collaborator

@evgenijolshanskij evgenijolshanskij left a comment

Choose a reason for hiding this comment

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

Refresh accessToken flow implemented wrong. For example, if a user is located on some auth page (e.g. /profile) and accessToken is expired, after token refresh the user is being taken on /login and then on the root (/).

Copy link
Collaborator

@evgenijolshanskij evgenijolshanskij left a comment

Choose a reason for hiding this comment

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

The yarn seed fails.

Copy link
Collaborator

@evgenijolshanskij evgenijolshanskij left a comment

Choose a reason for hiding this comment

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

I don't want REST Kit to be a dummy "copy/paste" of the existing one. During working on this Kit I would like to take into account all the drawbacks and make it better. I don't think that such a huge amount of components should be placed in one directory. We create a modular structure, thus modules can be split into smaller modules as well.

@Iwasherd Iwasherd assigned Iwasherd and unassigned Iwasherd May 29, 2019

import { AuthRoute, IfLoggedIn, IfNotLoggedIn, withLogout, WithLogoutProps } from '../containers/Auth';

export interface LoginSubmitProps {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest using types dir for not polluting index files.

Choose a reason for hiding this comment

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

Done

<AuthRoute exact path="/reset-password/:token" redirectOnLoggedIn redirect="/profile" component={ResetPassword} />
],
navItemRight: [
<IfLoggedIn key="/logout">
Copy link
Collaborator

Choose a reason for hiding this comment

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

All components should have been moved out of the index file.

Choose a reason for hiding this comment

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

Done

}

const UserAddView = ({ t, onSubmit }: UserAddViewProps) => {
const renderMetaData = () => (
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the point to duplicate this code snippet in ALL components?

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,7 @@
import { ActionFunction, ActionType } from '.';

const CLEAR_USER: ActionFunction<ActionType> = () => ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

I have never seen functions to be called like this. Could you please give me a link to this convention.

Copy link

Choose a reason for hiding this comment

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

Done

meta: string;
}

const MetaData = ({ title, meta }: MetaDataProps) => (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why MetaData is located in the user? Is it the only module that will use it?

rerenderApp();
};

class PageReloader extends React.Component<PageReloaderProps> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should stop using class-based components in favor of functional ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants