Improve typings for AuthFlow and mark some future TODOs

2024-11-23 00:22:57 +05:30 · 2024-08-28 13:07:23 +02:00 · 2024-08-28 13:07:23 +02:00 · ba2876e534
commit ba2876e534
parent 12f5e711c4
23 changed files with 148 additions and 156 deletions
--- a/packages/app/components/auth/PanelTransition.tsx
+++ b/packages/app/components/auth/PanelTransition.tsx
@ -180,8 +180,6 @@ class PanelTransition extends React.PureComponent<Props, State> {

        if (this.props.children) {
            return this.props.children;
-        } else if (!Title || !Body || !Footer || !Links) {
-            throw new Error('Title, Body, Footer and Links are required');
        }

        const {
--- a/packages/app/components/auth/actions.ts
+++ b/packages/app/components/auth/actions.ts
@ -182,7 +182,7 @@ export function register({
    );
 }

-export function activate({ key = '' }: { key: string }): AppAction<Promise<Account>> {
+export function activate(key: string): AppAction<Promise<Account>> {
    return wrapInLoader((dispatch) =>
        activateEndpoint(key)
            .then(authHandler(dispatch))
--- a/packages/app/services/authFlow/AbstractState.ts
+++ b/packages/app/services/authFlow/AbstractState.ts
@ -1,7 +1,8 @@
 /* eslint-disable @typescript-eslint/no-unused-vars */
+import State from './State';
 import { AuthContext } from 'app/services/authFlow';

-export default class AbstractState {
+export default class AbstractState implements State {
    resolve(context: AuthContext, payload: Record<string, any>): Promise<void> | void {}
    goBack(context: AuthContext): void {
        throw new Error('There is no way back');
--- a/packages/app/services/authFlow/ActivationState.test.ts
+++ b/packages/app/services/authFlow/ActivationState.test.ts
@ -57,9 +57,9 @@ describe('ActivationState', () => {

    describe('#resolve', () => {
        it('should call activate with payload', () => {
-            const payload = {};
+            const payload = { key: 'mock' };

-            expectRun(mock, 'activate', sinon.match.same(payload)).returns(new Promise(() => {}));
+            expectRun(mock, 'activate', sinon.match.same('mock')).returns(new Promise(() => {}));

            state.resolve(context, payload);
        });
@ -70,7 +70,7 @@ describe('ActivationState', () => {
            mock.expects('run').returns(promise);
            expectState(mock, CompleteState);

-            state.resolve(context, {});
+            state.resolve(context, { key: 'mock' });

            return promise;
        });
@ -81,7 +81,7 @@ describe('ActivationState', () => {
            mock.expects('run').returns(promise);
            mock.expects('setState').never();

-            state.resolve(context, {});
+            state.resolve(context, { key: 'mock' });

            return promise.catch(mock.verify.bind(mock));
        });
--- a/packages/app/services/authFlow/ActivationState.ts
+++ b/packages/app/services/authFlow/ActivationState.ts
@ -11,9 +11,9 @@ export default class ActivationState extends AbstractState {
        context.navigate(url);
    }

-    resolve(context: AuthContext, payload: Record<string, any>): Promise<void> | void {
+    resolve(context: AuthContext, payload: { key: string }): Promise<void> | void {
        context
-            .run('activate', payload)
+            .run('activate', payload.key)
            .then(() => context.setState(new CompleteState()))
            .catch((err = {}) => err.errors || logger.warn('Error activating account', err));
    }
--- a/packages/app/services/authFlow/AuthFlow.test.ts
+++ b/packages/app/services/authFlow/AuthFlow.test.ts
@ -255,11 +255,6 @@ describe('AuthFlow', () => {
            // @ts-ignore
            return expect(flow.run('test'), 'to be fulfilled with', expected);
        });
-
-        it('throws when running unexisted action', () => {
-            // @ts-ignore
-            expect(() => flow.run('123'), 'to throw', 'Action 123 does not exists');
-        });
    });

    describe('#goBack', () => {
--- a/packages/app/services/authFlow/AuthFlow.ts
+++ b/packages/app/services/authFlow/AuthFlow.ts
@ -1,7 +1,15 @@
 import { browserHistory } from 'app/services/history';
 import logger from 'app/services/logger';
 import localStorage from 'app/services/localStorage';
-import { Store, State as RootState } from 'app/types';
+import { Store, State as RootState, Dispatch } from 'app/types';
+import {
+    activate as activateAccount,
+    authenticate,
+    logoutAll as logout,
+    remove as removeAccount,
+} from 'app/components/accounts/actions';
+import * as actions from 'app/components/auth/actions';
+import { updateUser } from 'app/components/user/actions';

 import RegisterState from './RegisterState';
 import LoginState from './LoginState';
@ -12,7 +20,7 @@ import ActivationState from './ActivationState';
 import CompleteState from './CompleteState';
 import ChooseAccountState from './ChooseAccountState';
 import ResendActivationState from './ResendActivationState';
-import AbstractState from './AbstractState';
+import State from './State';

 type Request = {
    path: string;
@ -20,53 +28,53 @@ type Request = {
    params: Record<string, any>;
 };

-// TODO: temporary added to improve typing without major refactoring
-type ActionId =
-    | 'updateUser'
-    | 'authenticate'
-    | 'activateAccount'
-    | 'removeAccount'
-    | 'logout'
-    | 'goBack'
-    | 'redirect'
-    | 'login'
-    | 'acceptRules'
-    | 'forgotPassword'
-    | 'recoverPassword'
-    | 'register'
-    | 'activate'
-    | 'resendActivation'
-    | 'contactUs'
-    | 'setLogin'
-    | 'setAccountSwitcher'
-    | 'setErrors'
-    | 'clearErrors'
-    | 'oAuthValidate'
-    | 'oAuthComplete'
-    | 'setClient'
-    | 'resetOAuth'
-    | 'resetAuth'
-    | 'setOAuthRequest'
-    | 'setOAuthCode'
-    | 'requirePermissionsAccept'
-    | 'setScopes'
-    | 'setLoadingState';
+export const availableActions = {
+    updateUser,
+    authenticate,
+    activateAccount,
+    removeAccount,
+    logout,
+    goBack: actions.goBack,
+    redirect: actions.redirect,
+    login: actions.login,
+    acceptRules: actions.acceptRules,
+    forgotPassword: actions.forgotPassword,
+    recoverPassword: actions.recoverPassword,
+    register: actions.register,
+    activate: actions.activate,
+    resendActivation: actions.resendActivation,
+    contactUs: actions.contactUs,
+    setLogin: actions.setLogin,
+    setAccountSwitcher: actions.setAccountSwitcher,
+    setErrors: actions.setErrors,
+    clearErrors: actions.clearErrors,
+    oAuthValidate: actions.oAuthValidate,
+    oAuthComplete: actions.oAuthComplete,
+    setClient: actions.setClient,
+    resetOAuth: actions.resetOAuth,
+    resetAuth: actions.resetAuth,
+    setOAuthRequest: actions.setOAuthRequest,
+    setOAuthCode: actions.setOAuthCode,
+    requirePermissionsAccept: actions.requirePermissionsAccept,
+    setScopes: actions.setScopes,
+    setLoadingState: actions.setLoadingState,
+};
+
+type ActionId = keyof typeof availableActions;

 export interface AuthContext {
-    run(actionId: ActionId, payload?: any): Promise<any>;
-    setState(newState: AbstractState): Promise<void> | void;
+    run<T extends ActionId>(actionId: T, payload?: Parameters<typeof availableActions[T]>[0]): Promise<any>; // TODO: can't find a way to explain to TS the returned type
+    setState(newState: State): Promise<void> | void; // TODO: always return promise
    getState(): RootState;
    navigate(route: string, options?: { replace?: boolean }): void;
    getRequest(): Request;
-    prevState: AbstractState;
+    prevState: State;
 }

-export type ActionsDict = Record<ActionId, (action: any) => Record<string, any>>;
-
 export default class AuthFlow implements AuthContext {
-    actions: Readonly<ActionsDict>;
-    state: AbstractState;
-    prevState: AbstractState;
+    actions: Readonly<typeof availableActions>;
+    state: State;
+    prevState: State;
    /**
     * A callback from router, that allows to replace (perform redirect) route
     * during route transition
@ -76,10 +84,10 @@ export default class AuthFlow implements AuthContext {
    navigate: (route: string, options: { replace?: boolean }) => void;
    currentRequest: Partial<Request> = {};
    oAuthStateRestored = false;
-    dispatch: (action: Record<string, any>) => void;
+    dispatch: Dispatch;
    getState: () => RootState;

-    constructor(actions: ActionsDict) {
+    constructor(actions: typeof availableActions) {
        this.actions = Object.freeze(actions);
    }

@ -106,11 +114,11 @@ export default class AuthFlow implements AuthContext {
        this.dispatch = store.dispatch.bind(store);
    }

-    resolve(payload: { [key: string]: any } = {}) {
+    resolve(payload: Record<string, any> = {}) {
        this.state.resolve(this, payload);
    }

-    reject(payload: { [key: string]: any } = {}) {
+    reject(payload: Record<string, any> = {}) {
        this.state.reject(this, payload);
    }

@ -118,17 +126,12 @@ export default class AuthFlow implements AuthContext {
        this.state.goBack(this);
    }

-    run(actionId: ActionId, payload?: Record<string, any>): Promise<any> {
-        const action = this.actions[actionId];
-
-        if (!action) {
-            throw new Error(`Action ${actionId} does not exists`);
-        }
-
-        return Promise.resolve(this.dispatch(action(payload)));
+    run<T extends ActionId>(actionId: T, payload?: Parameters<typeof availableActions[T]>[0]): Promise<any> {
+        // @ts-ignore the extended version of redux with thunk will return the correct promise
+        return Promise.resolve(this.dispatch(this.actions[actionId](payload)));
    }

-    setState(state: AbstractState) {
+    setState(state: State) {
        if (!state) {
            throw new Error('State is required');
        }
--- a/packages/app/services/authFlow/ChooseAccountState.test.ts
+++ b/packages/app/services/authFlow/ChooseAccountState.test.ts
@ -1,12 +1,22 @@
 import expect from 'app/test/unexpected';
 import sinon, { SinonMock } from 'sinon';

+import { Account } from 'app/components/accounts';
 import ChooseAccountState from 'app/services/authFlow/ChooseAccountState';
 import CompleteState from 'app/services/authFlow/CompleteState';
 import LoginState from 'app/services/authFlow/LoginState';

 import { bootstrap, expectState, expectNavigate, expectRun, MockedAuthContext } from './helpers';

+const mockAccount: Account = {
+    id: 1,
+    username: '',
+    email: '',
+    token: '',
+    refreshToken: '',
+    isDeleted: false,
+};
+
 describe('ChooseAccountState', () => {
    let state: ChooseAccountState;
    let context: MockedAuthContext;
@ -52,17 +62,11 @@ describe('ChooseAccountState', () => {

    describe('#resolve', () => {
        it('should transition to complete if an existing account was chosen', () => {
-            expectRun(
-                mock,
-                'authenticate',
-                sinon.match({
-                    id: 123,
-                }),
-            ).returns(Promise.resolve());
+            expectRun(mock, 'authenticate', sinon.match(mockAccount)).returns(Promise.resolve());
            expectRun(mock, 'setAccountSwitcher', false);
            expectState(mock, CompleteState);

-            return expect(state.resolve(context, { id: 123 }), 'to be fulfilled');
+            return expect(state.resolve(context, mockAccount), 'to be fulfilled');
        });

        it('should transition to login if user wants to add new account', () => {
@ -71,6 +75,7 @@ describe('ChooseAccountState', () => {
            expectState(mock, LoginState);

            // Assert nothing returned
+            // @ts-ignore
            return expect(state.resolve(context, {}), 'to be undefined');
        });
    });
--- a/packages/app/services/authFlow/ChooseAccountState.ts
+++ b/packages/app/services/authFlow/ChooseAccountState.ts
@ -16,9 +16,11 @@ export default class ChooseAccountState extends AbstractState {
        }
    }

-    resolve(context: AuthContext, payload: Account | Record<string, any>): Promise<void> | void {
+    // This method might be called with an empty object to mention that user wants to login into a new account.
+    // Currently, I can't correctly provide typing since there is no type for an empty object.
+    // So if there is no `id` property, it's an empty object
+    resolve(context: AuthContext, payload: Account): Promise<void> | void {
        if (payload.id) {
-            // payload is Account
            return context
                .run('authenticate', payload)
                .then(() => context.run('setAccountSwitcher', false))
--- a/packages/app/services/authFlow/CompleteState.ts
+++ b/packages/app/services/authFlow/CompleteState.ts
@ -106,7 +106,7 @@ export default class CompleteState extends AbstractState {
            return context.run('oAuthComplete', data).then(
                (resp: { redirectUri: string }) => {
                    // TODO: пусть в стейт попадает флаг или тип авторизации
-                    // вместо волшебства над редирект урлой
+                    //       вместо волшебства над редирект урлой
                    if (resp.redirectUri.includes('static_page')) {
                        context.setState(new FinishState());
                    } else {
--- a/packages/app/services/authFlow/ForgotPasswordState.test.ts
+++ b/packages/app/services/authFlow/ForgotPasswordState.test.ts
@ -43,7 +43,7 @@ describe('ForgotPasswordState', () => {
                }),
            ).returns(new Promise(() => {}));

-            state.resolve(context, { login: expectedLogin });
+            state.resolve(context, { login: expectedLogin, captcha: '' });
        });

        it('should transition to recoverPassword state on success', () => {
@ -53,7 +53,7 @@ describe('ForgotPasswordState', () => {
            mock.expects('run').twice().returns(promise);
            expectState(mock, RecoverPasswordState);

-            state.resolve(context, { login: expectedLogin });
+            state.resolve(context, { login: expectedLogin, captcha: '' });

            return promise;
        });
@ -66,7 +66,7 @@ describe('ForgotPasswordState', () => {
            expectState(mock, RecoverPasswordState);
            mock.expects('run').withArgs('setLogin', expectedLogin);

-            state.resolve(context, { login: expectedLogin });
+            state.resolve(context, { login: expectedLogin, captcha: '' });

            return promise;
        });
--- a/packages/app/services/authFlow/ForgotPasswordState.ts
+++ b/packages/app/services/authFlow/ForgotPasswordState.ts
@ -10,12 +10,7 @@ export default class ForgotPasswordState extends AbstractState {
        context.navigate('/forgot-password');
    }

-    resolve(
-        context: AuthContext,
-        payload: {
-            login?: string;
-        } = {},
-    ): Promise<void> | void {
+    resolve(context: AuthContext, payload: { login: string; captcha: string }): Promise<void> | void {
        context
            .run('forgotPassword', payload)
            .then(() => {
--- a/packages/app/services/authFlow/MfaState.ts
+++ b/packages/app/services/authFlow/MfaState.ts
@ -23,6 +23,7 @@ export default class MfaState extends AbstractState {

        return context
            .run('login', {
+                // @ts-ignore there will be no invalid value
                login,
                password,
                totp,
--- a/packages/app/services/authFlow/OAuthState.ts
+++ b/packages/app/services/authFlow/OAuthState.ts
@ -9,13 +9,13 @@ export default class OAuthState extends AbstractState {
        return context
            .run('oAuthValidate', {
                clientId: query.get('client_id') || params.clientId,
-                redirectUrl: query.get('redirect_uri'),
-                responseType: query.get('response_type'),
-                description: query.get('description'),
+                redirectUrl: query.get('redirect_uri')!,
+                responseType: query.get('response_type')!,
+                description: query.get('description')!,
                scope: (query.get('scope') || '').replace(/,/g, ' '),
-                prompt: query.get('prompt'),
-                loginHint: query.get('login_hint'),
-                state: query.get('state'),
+                prompt: query.get('prompt')!,
+                loginHint: query.get('login_hint')!,
+                state: query.get('state')!,
            })
            .then(() => context.setState(new CompleteState()));
    }
--- a/packages/app/services/authFlow/PasswordState.ts
+++ b/packages/app/services/authFlow/PasswordState.ts
@ -39,6 +39,7 @@ export default class PasswordState extends AbstractState {
            .run('login', {
                password,
                rememberMe,
+                // @ts-ignore there will be no null value
                login,
            })
            .then(() => {
--- a/packages/app/services/authFlow/RecoverPasswordState.test.ts
+++ b/packages/app/services/authFlow/RecoverPasswordState.test.ts
@ -48,7 +48,7 @@ describe('RecoverPasswordState', () => {
    describe('#resolve', () => {
        it('should call recoverPassword with provided payload', () => {
            const expectedPayload = {
-                key: 123,
+                key: '123',
                newPassword: '123',
                newRePassword: '123',
            };
@ -64,7 +64,7 @@ describe('RecoverPasswordState', () => {
            mock.expects('run').returns(promise);
            expectState(mock, CompleteState);

-            state.resolve(context, {});
+            state.resolve(context, { key: '', newPassword: '', newRePassword: '' });

            return promise;
        });
@ -75,7 +75,7 @@ describe('RecoverPasswordState', () => {
            mock.expects('run').returns(promise);
            mock.expects('setState').never();

-            state.resolve(context, {});
+            state.resolve(context, { key: '', newPassword: '', newRePassword: '' });

            return promise.catch(mock.verify.bind(mock));
        });
--- a/packages/app/services/authFlow/RecoverPasswordState.ts
+++ b/packages/app/services/authFlow/RecoverPasswordState.ts
@ -13,7 +13,10 @@ export default class RecoverPasswordState extends AbstractState {
        context.navigate(url);
    }

-    resolve(context: AuthContext, payload: Record<string, any>): Promise<void> | void {
+    resolve(
+        context: AuthContext,
+        payload: { key: string; newPassword: string; newRePassword: string },
+    ): Promise<void> | void {
        context
            .run('recoverPassword', payload)
            .then(() => context.setState(new CompleteState()))
--- a/packages/app/services/authFlow/RegisterState.test.ts
+++ b/packages/app/services/authFlow/RegisterState.test.ts
@ -12,6 +12,15 @@ describe('RegisterState', () => {
    let context: MockedAuthContext;
    let mock: SinonMock;

+    const mockPayload = {
+        username: '',
+        email: '',
+        password: '',
+        rePassword: '',
+        rulesAgreement: true,
+        captcha: '',
+    };
+
    beforeEach(() => {
        state = new RegisterState();

@ -38,21 +47,18 @@ describe('RegisterState', () => {

    describe('#resolve', () => {
        it('should register on resolve', () => {
-            const payload = {};
+            expectRun(mock, 'register', sinon.match.same(mockPayload)).returns(new Promise(() => {}));

-            expectRun(mock, 'register', sinon.match.same(payload)).returns(new Promise(() => {}));
-
-            state.resolve(context, payload);
+            state.resolve(context, mockPayload);
        });

        it('should transition to complete after register', () => {
-            const payload = {};
            const promise = Promise.resolve();

            mock.expects('run').returns(promise);
            expectState(mock, CompleteState);

-            state.resolve(context, payload);
+            state.resolve(context, mockPayload);

            return promise;
        });
@ -63,7 +69,7 @@ describe('RegisterState', () => {
            mock.expects('run').returns(promise);
            mock.expects('setState').never();

-            state.resolve(context, {});
+            state.resolve(context, mockPayload);

            return promise.catch(mock.verify.bind(mock));
        });
--- a/packages/app/services/authFlow/RegisterState.ts
+++ b/packages/app/services/authFlow/RegisterState.ts
@ -11,7 +11,17 @@ export default class RegisterState extends AbstractState {
        context.navigate('/register');
    }

-    resolve(context: AuthContext, payload: Record<string, any>): Promise<void> | void {
+    resolve(
+        context: AuthContext,
+        payload: {
+            email: string;
+            username: string;
+            password: string;
+            rePassword: string;
+            captcha: string;
+            rulesAgreement: boolean;
+        },
+    ): Promise<void> | void {
        context
            .run('register', payload)
            .then(() => context.setState(new CompleteState()))
--- a/packages/app/services/authFlow/ResendActivationState.test.ts
+++ b/packages/app/services/authFlow/ResendActivationState.test.ts
@ -11,6 +11,11 @@ describe('ResendActivationState', () => {
    let context: MockedAuthContext;
    let mock: SinonMock;

+    const mockPayload = {
+        email: 'foo@bar.com',
+        captcha: '',
+    };
+
    beforeEach(() => {
        state = new ResendActivationState();

@ -52,11 +57,9 @@ describe('ResendActivationState', () => {

    describe('#resolve', () => {
        it('should call resendActivation with payload', () => {
-            const payload = { email: 'foo@bar.com' };
+            expectRun(mock, 'resendActivation', sinon.match.same(mockPayload)).returns(new Promise(() => {}));

-            expectRun(mock, 'resendActivation', sinon.match.same(payload)).returns(new Promise(() => {}));
-
-            state.resolve(context, payload);
+            state.resolve(context, mockPayload);
        });

        it('should transition to complete state on success', () => {
@ -65,7 +68,7 @@ describe('ResendActivationState', () => {
            mock.expects('run').returns(promise);
            expectState(mock, ActivationState);

-            state.resolve(context, {});
+            state.resolve(context, mockPayload);

            return promise;
        });
@ -76,7 +79,7 @@ describe('ResendActivationState', () => {
            mock.expects('run').returns(promise);
            mock.expects('setState').never();

-            state.resolve(context, {});
+            state.resolve(context, mockPayload);

            return promise.catch(mock.verify.bind(mock));
        });
--- a/packages/app/services/authFlow/ResendActivationState.ts
+++ b/packages/app/services/authFlow/ResendActivationState.ts
@ -10,7 +10,7 @@ export default class ResendActivationState extends AbstractState {
        context.navigate('/resend-activation');
    }

-    resolve(context: AuthContext, payload: Record<string, any>): Promise<void> | void {
+    resolve(context: AuthContext, payload: { email: string; captcha: string }): Promise<void> | void {
        context
            .run('resendActivation', payload)
            .then(() => context.setState(new ActivationState()))
--- a/packages/app/services/authFlow/State.ts
+++ b/packages/app/services/authFlow/State.ts
@ -0,0 +1,10 @@
+import { AuthContext } from 'app/services/authFlow';
+
+export default interface State {
+    resolve(context: AuthContext, payload: Record<string, any>): Promise<void> | void;
+    goBack(context: AuthContext): void;
+    // TODO: remove this method and handle next state resolution via Resolve method
+    reject(context: AuthContext, payload?: Record<string, any>): void;
+    enter(context: AuthContext): Promise<void> | void;
+    leave(context: AuthContext): void;
+}
--- a/packages/app/services/authFlow/index.ts
+++ b/packages/app/services/authFlow/index.ts
@ -1,45 +1,4 @@
-import * as actions from 'app/components/auth/actions';
-import { updateUser } from 'app/components/user/actions';
-import {
-    authenticate,
-    logoutAll as logout,
-    remove as removeAccount,
-    activate as activateAccount,
-} from 'app/components/accounts/actions';
-
-import AuthFlow, { ActionsDict, AuthContext as TAuthContext } from './AuthFlow';
-
-const availableActions: ActionsDict = {
-    updateUser,
-    authenticate,
-    activateAccount,
-    removeAccount,
-    logout,
-    goBack: actions.goBack,
-    redirect: actions.redirect,
-    login: actions.login,
-    acceptRules: actions.acceptRules,
-    forgotPassword: actions.forgotPassword,
-    recoverPassword: actions.recoverPassword,
-    register: actions.register,
-    activate: actions.activate,
-    resendActivation: actions.resendActivation,
-    contactUs: actions.contactUs,
-    setLogin: actions.setLogin,
-    setAccountSwitcher: actions.setAccountSwitcher,
-    setErrors: actions.setErrors,
-    clearErrors: actions.clearErrors,
-    oAuthValidate: actions.oAuthValidate,
-    oAuthComplete: actions.oAuthComplete,
-    setClient: actions.setClient,
-    resetOAuth: actions.resetOAuth,
-    resetAuth: actions.resetAuth,
-    setOAuthRequest: actions.setOAuthRequest,
-    setOAuthCode: actions.setOAuthCode,
-    requirePermissionsAccept: actions.requirePermissionsAccept,
-    setScopes: actions.setScopes,
-    setLoadingState: actions.setLoadingState,
-};
+import AuthFlow, { availableActions, AuthContext as TAuthContext } from './AuthFlow';

 export type AuthContext = TAuthContext;
 export default new AuthFlow(availableActions);