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

Remove immutable from status and auth #4727

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/netlify-cms-core/src/actions/status.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { State } from '../types/redux';
import { currentBackend } from '../backend';
import { ThunkDispatch } from 'redux-thunk';
import { AnyAction } from 'redux';
import { actions as notifActions } from 'redux-notifications';
import { State } from '../types/redux';
import { currentBackend } from '../backend';

const { notifSend, notifDismiss } = notifActions;

Expand Down Expand Up @@ -37,7 +37,7 @@ export function checkBackendStatus() {
return async (dispatch: ThunkDispatch<State, {}, AnyAction>, getState: () => State) => {
try {
const state = getState();
if (state.status.get('isFetching')) {
if (state.status.isFetching) {
return;
}

Expand Down
11 changes: 5 additions & 6 deletions packages/netlify-cms-core/src/components/App/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ const RouteInCollection = ({ collections, render, ...props }) => {

class App extends React.Component {
static propTypes = {
auth: ImmutablePropTypes.map,
auth: PropTypes.object.isRequired,
config: ImmutablePropTypes.map,
collections: ImmutablePropTypes.orderedMap,
loadConfig: PropTypes.func.isRequired,
loginUser: PropTypes.func.isRequired,
logoutUser: PropTypes.func.isRequired,
user: ImmutablePropTypes.map,
user: PropTypes.object,
isFetching: PropTypes.bool.isRequired,
publishMode: PropTypes.oneOf([SIMPLE, EDITORIAL_WORKFLOW]),
siteId: PropTypes.string,
Expand Down Expand Up @@ -129,9 +129,8 @@ class App extends React.Component {
<Notifs CustomComponent={Toast} />
{React.createElement(backend.authComponent(), {
onLogin: this.handleLogin.bind(this),
error: auth && auth.get('error'),
isFetching: auth && auth.get('isFetching'),
inProgress: (auth && auth.get('isFetching')) || false,
error: auth.error,
inProgress: auth.isFetching,
siteId: this.props.config.getIn(['backend', 'site_domain']),
base_url: this.props.config.getIn(['backend', 'base_url'], null),
authEndpoint: this.props.config.getIn(['backend', 'auth_endpoint']),
Expand Down Expand Up @@ -262,7 +261,7 @@ class App extends React.Component {

function mapStateToProps(state) {
const { auth, config, collections, globalUI, mediaLibrary } = state;
const user = auth && auth.get('user');
const user = auth.user;
const isFetching = globalUI.get('isFetching');
const publishMode = config && config.get('publish_mode');
const useMediaLibrary = !mediaLibrary.get('externalLibrary');
Expand Down
4 changes: 2 additions & 2 deletions packages/netlify-cms-core/src/components/App/Header.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ const AppHeaderNavList = styled.ul`

class Header extends React.Component {
static propTypes = {
user: ImmutablePropTypes.map.isRequired,
user: PropTypes.object.isRequired,
collections: ImmutablePropTypes.orderedMap.isRequired,
onCreateEntryClick: PropTypes.func.isRequired,
onLogoutClick: PropTypes.func.isRequired,
Expand Down Expand Up @@ -216,7 +216,7 @@ class Header extends React.Component {
<SettingsDropdown
displayUrl={displayUrl}
isTestRepo={isTestRepo}
imageUrl={user.get('avatar_url')}
imageUrl={user?.avatar_url}
onLogoutClick={onLogoutClick}
/>
</AppHeaderActions>
Expand Down
4 changes: 2 additions & 2 deletions packages/netlify-cms-core/src/components/Editor/Editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export class Editor extends React.Component {
deployPreview: ImmutablePropTypes.map,
loadDeployPreview: PropTypes.func.isRequired,
currentStatus: PropTypes.string,
user: ImmutablePropTypes.map.isRequired,
user: PropTypes.object,
location: PropTypes.shape({
pathname: PropTypes.string,
search: PropTypes.string,
Expand Down Expand Up @@ -432,7 +432,7 @@ function mapStateToProps(state, ownProps) {
const newEntry = ownProps.newRecord === true;
const fields = selectFields(collection, slug);
const entry = newEntry ? null : selectEntry(state, collectionName, slug);
const user = auth && auth.get('user');
const user = auth.user;
const hasChanged = entryDraft.get('hasChanged');
const displayUrl = config.get('display_url');
const hasWorkflow = config.get('publish_mode') === EDITORIAL_WORKFLOW;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ EditorInterface.propTypes = {
unPublish: PropTypes.func.isRequired,
onDuplicate: PropTypes.func.isRequired,
onChangeStatus: PropTypes.func.isRequired,
user: ImmutablePropTypes.map.isRequired,
user: PropTypes.object,
hasChanged: PropTypes.bool,
displayUrl: PropTypes.string,
hasWorkflow: PropTypes.bool,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ class EditorToolbar extends React.Component {
onDuplicate: PropTypes.func.isRequired,
onPublishAndNew: PropTypes.func.isRequired,
onPublishAndDuplicate: PropTypes.func.isRequired,
user: ImmutablePropTypes.map.isRequired,
user: PropTypes.object,
hasChanged: PropTypes.bool,
displayUrl: PropTypes.string,
collection: ImmutablePropTypes.map.isRequired,
Expand Down Expand Up @@ -606,7 +606,7 @@ class EditorToolbar extends React.Component {
<ToolbarSectionMeta>
<SettingsDropdown
displayUrl={displayUrl}
imageUrl={user.get('avatar_url')}
imageUrl={user?.avatar_url}
onLogoutClick={onLogoutClick}
/>
</ToolbarSectionMeta>
Expand Down
24 changes: 15 additions & 9 deletions packages/netlify-cms-core/src/reducers/__tests__/auth.spec.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,38 @@
import { fromJS } from 'immutable';
import { authenticating, authenticate, authError, logout } from '../../actions/auth';
import auth, { defaultState } from '../auth';

describe('auth', () => {
it('should handle an empty state', () => {
// eslint-disable-next-line @typescript-eslint/ban-ts-ignore
// @ts-ignore
// @ts-ignore auth reducer doesn't accept empty action
expect(auth(undefined, {})).toEqual(defaultState);
});

it('should handle an authentication request', () => {
expect(auth(undefined, authenticating())).toEqual(defaultState.set('isFetching', true));
expect(auth(undefined, authenticating())).toEqual({
...defaultState,
isFetching: true,
});
});

it('should handle authentication', () => {
const user = { name: 'joe', token: 'token' };
expect(auth(undefined, authenticate(user))).toEqual(defaultState.set('user', fromJS(user)));
expect(auth(undefined, authenticate(user))).toEqual({
...defaultState,
user,
});
});

it('should handle an authentication error', () => {
expect(auth(undefined, authError(new Error('Bad credentials')))).toEqual(
defaultState.set('error', 'Error: Bad credentials'),
);
expect(auth(undefined, authError(new Error('Bad credentials')))).toEqual({
...defaultState,
error: 'Error: Bad credentials',
});
});

it('should handle logout', () => {
const user = { name: 'joe', token: 'token' };
const newState = auth(defaultState.set('user', fromJS(user)), logout());
expect(newState.get('user')).toBeUndefined();
const newState = auth({ ...defaultState, user }, logout());
expect(newState.user).toBeUndefined();
});
});
34 changes: 18 additions & 16 deletions packages/netlify-cms-core/src/reducers/auth.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fromJS } from 'immutable';
import { produce } from 'immer';
import { User } from 'netlify-cms-lib-util';
import {
AUTH_REQUEST,
Expand All @@ -8,35 +8,37 @@ import {
LOGOUT,
AuthAction,
} from '../actions/auth';
import { StaticallyTypedRecord } from '../types/immutable';

export type Auth = StaticallyTypedRecord<{
export type Auth = {
isFetching: boolean;
user: StaticallyTypedRecord<User> | undefined;
user: User | undefined;
error: string | undefined;
}>;
};

export const defaultState = fromJS({
export const defaultState: Auth = {
isFetching: false,
user: undefined,
error: undefined,
}) as Auth;
};

const auth = (state = defaultState, action: AuthAction) => {
const auth = produce((state: Auth, action: AuthAction) => {
switch (action.type) {
case AUTH_REQUEST:
return state.set('isFetching', true);
state.isFetching = true;
break;
case AUTH_SUCCESS:
return state.set('user', fromJS(action.payload));
state.user = action.payload;
break;
case AUTH_FAILURE:
return state.set('error', action.payload && action.payload.toString());
state.error = action.payload && action.payload.toString();
break;
case AUTH_REQUEST_DONE:
return state.set('isFetching', false);
state.isFetching = false;
break;
case LOGOUT:
return state.set('user', undefined).set('isFetching', false);
default:
return state;
state.user = undefined;
state.isFetching = false;
}
};
}, defaultState);

export default auth;
45 changes: 18 additions & 27 deletions packages/netlify-cms-core/src/reducers/status.ts
Original file line number Diff line number Diff line change
@@ -1,46 +1,37 @@
import { fromJS } from 'immutable';
import { produce } from 'immer';
import { STATUS_REQUEST, STATUS_SUCCESS, STATUS_FAILURE, StatusAction } from '../actions/status';
import { StaticallyTypedRecord } from '../types/immutable';

export type Status = StaticallyTypedRecord<{
export type Status = {
isFetching: boolean;
status: StaticallyTypedRecord<{
auth: StaticallyTypedRecord<{ status: boolean }>;
api: StaticallyTypedRecord<{ status: boolean; statusPage: string }>;
}>;
status: {
auth: { status: boolean };
api: { status: boolean; statusPage: string };
};
error: Error | undefined;
}>;
};

const defaultState = fromJS({
const defaultState: Status = {
isFetching: false,
status: {
auth: { status: true },
api: { status: true, statusPage: '' },
},
error: undefined,
}) as Status;
};

const status = (state = defaultState, action: StatusAction) => {
const status = produce((state: Status, action: StatusAction) => {
switch (action.type) {
case STATUS_REQUEST:
return state.set('isFetching', true);
state.isFetching = true;
break;
case STATUS_SUCCESS:
return state.withMutations(map => {
map.set('isFetching', false);
map.set('status', fromJS(action.payload.status));
});
state.isFetching = false;
state.status = action.payload.status;
break;
case STATUS_FAILURE:
return state.withMutations(map => {
map.set('isFetching', false);
map.set('error', action.payload.error);
});
default:
return state;
state.isFetching = false;
state.error = action.payload.error;
}
};

export const selectStatus = (status: Status) => {
return status.get('status').toJS();
};
}, defaultState);

export default status;