Redux: Modernize Redux store

This commit modernizes the Redux store to bring it in line with the
current recommended best practices/patterns. It is possible because of a
recent previous commit that added Redux Toolkit as a dependency.

For detailed information on what modern Redux entails, see the Redux docs:
https://redux.js.org/introduction/getting-started#learn-modern-redux-livestream

Practically speaking, this means a huge reduction in boilerplate code.
Maintaining and extending the code will be much easier. All Redux logic
is now encapsulated by 'slices'. Reducers are defined in the slices, and
action creators and action types are generated automatically.

Redux Toolkit includes Immer, which greatly simplifies writing reducer logic
much simpler - state updates in Redux must be immutable, but working
with Javascript objects in an immutable fashion is clumsy, requiring
gratuitious use of the spread ... operator. With Immer, the state can be
updated as if mutable updates were allowed. Reducer logic has been
changed to take advantage of this in this commit.

This commit also removes a large amount of unused code. Fingers crossed
that nothing breaks. The tests do pass, afterall...

One other minor change... composesGet was renamed to fetchComposes and
composeGetStatus was renamed to fetchComposeStatus. This is in line with
the Redux documentation examples.
This commit is contained in:
lucasgarfield 2022-08-31 16:57:54 +02:00 committed by Sanne Raymaekers
parent 1276b4ae9c
commit a17a759d5e
11 changed files with 76 additions and 374 deletions

View file

@ -19,12 +19,12 @@ import {
fileSystemConfigurationValidator,
targetEnvironmentValidator,
} from './validators';
import { composeAdded } from '../../store/composesSlice';
import DocumentationButton from '../sharedComponents/DocumentationButton';
import './CreateImageWizard.scss';
import api from '../../api';
import { UNIT_GIB, UNIT_KIB, UNIT_MIB } from '../../constants';
import isRhel from '../../Utilities/isRhel';
import { composeAdded } from '../../store/actions/actions';
const handleKeyDown = (e, handleClose) => {
if (e.key === 'Escape') {
@ -427,14 +427,14 @@ const CreateImageWizard = () => {
requests.map((request) =>
api.composeImage(request).then((response) => {
dispatch(
composeAdded(
{
composeAdded({
compose: {
...response,
request,
image_status: { status: 'pending' },
},
true
)
insert: true,
})
);
})
)

View file

@ -33,7 +33,7 @@ import Target from './Target';
import ImageLink from './ImageLink';
import ErrorDetails from './ImageBuildErrorDetails';
import DocumentationButton from '../sharedComponents/DocumentationButton';
import { composeGetStatus, composesGet } from '../../store/actions/actions';
import { fetchComposes, fetchComposeStatus } from '../../store/actions/actions';
const ImagesTable = () => {
const [page, setPage] = useState(1);
@ -67,13 +67,13 @@ const ImagesTable = () => {
return;
}
dispatch(composeGetStatus(id));
dispatch(fetchComposeStatus(id));
});
};
/* Get all composes once on mount */
useEffect(() => {
dispatch(composesGet(perPage, 0));
dispatch(fetchComposes(perPage, 0));
}, []);
/* Reset the polling each time the composes in the store are updated */
@ -89,7 +89,7 @@ const ImagesTable = () => {
if (composes.count > composes.allIds.length) {
const pageIndex = page - 1;
const offset = pageIndex * perPage;
dispatch(composesGet(perPage, offset));
dispatch(fetchComposes(perPage, offset));
}
setPage(page);
@ -102,7 +102,7 @@ const ImagesTable = () => {
composes.count > composes.allIds.length &&
perPage > composes.allIds.length
) {
dispatch(composesGet(perPage, 0));
dispatch(fetchComposes(perPage, 0));
}
// page should be reset to the first page when the page size is changed.

View file

@ -1,158 +1,27 @@
import {
composeAdded,
composesUpdatedCount,
composeUpdatedStatus,
} from '../composesSlice';
import api from '../../api';
import types from '../types';
function composeUpdated(compose) {
return {
type: types.COMPOSE_UPDATED,
payload: { compose },
};
}
export const composeFailed = (error) => ({
type: types.COMPOSE_FAILED,
payload: { error },
});
export const composeAdded = (compose, insert) => ({
type: types.COMPOSE_ADDED,
payload: { compose, insert },
});
export const composeStart = (composeRequest) => async (dispatch) => {
// response will be of the format {id: ''}
const request = api.composeImage(composeRequest);
return request
.then((response) => {
// add the compose id to the compose object to provide access to the id if iterating through
// composes and add an image status of 'pending' alongside the compose request.
const compose = Object.assign(
{},
response,
{ request: composeRequest },
{ image_status: { status: 'pending' } }
);
dispatch(composeAdded(compose, true));
})
.catch((err) => {
if (err.response.status === 500) {
dispatch(composeFailed('Error: Something went wrong serverside'));
} else {
dispatch(composeFailed('Error: Something went wrong with the compose'));
}
});
};
export const composeUpdatedStatus = (id, status) => ({
type: types.COMPOSE_UPDATED_STATUS,
payload: { id, status },
});
export const composeGetStatus = (id) => async (dispatch) => {
export const fetchComposeStatus = (id) => async (dispatch) => {
const request = await api.getComposeStatus(id);
dispatch(composeUpdatedStatus(id, request.image_status));
dispatch(
composeUpdatedStatus({
id,
status: request.image_status,
})
);
};
export const composesUpdatedCount = (count) => ({
type: types.COMPOSES_UPDATED_COUNT,
payload: { count },
});
export const composesGet = (limit, offset) => async (dispatch) => {
export const fetchComposes = (limit, offset) => async (dispatch) => {
const request = await api.getComposes(limit, offset);
request.data.map((compose) => {
dispatch(composeAdded(compose, false));
dispatch(composeGetStatus(compose.id));
dispatch(composeAdded({ compose, insert: false }));
dispatch(fetchComposeStatus(compose.id));
});
dispatch(composesUpdatedCount(request.meta.count));
dispatch(composesUpdatedCount({ count: request.meta.count }));
};
function setRelease({ arch, distro }) {
return {
type: types.SET_RELEASE,
payload: {
arch,
distro,
},
};
}
function setUploadDestinations({ aws, azure, google }) {
return {
type: types.SET_UPLOAD_DESTINATIONS,
payload: {
aws,
azure,
google,
},
};
}
function setUploadAWS({ shareWithAccounts }) {
return {
type: types.SET_UPLOAD_AWS,
payload: {
shareWithAccounts,
},
};
}
function setUploadAzure({ tenantId, subscriptionId, resourceGroup }) {
return {
type: types.SET_UPLOAD_AZURE,
payload: {
tenantId,
subscriptionId,
resourceGroup,
},
};
}
function setUploadGoogle({ accountType, shareWithAccounts }) {
return {
type: types.SET_UPLOAD_GOOGLE,
payload: {
accountType,
shareWithAccounts,
},
};
}
function setSelectedPackages(selectedPackages) {
return {
type: types.SET_SELECTED_PACKAGES,
payload: selectedPackages,
};
}
function setSubscription({ activationKey, insights, organization }) {
return {
type: types.SET_SUBSCRIPTION,
payload: {
activationKey,
insights,
organization,
},
};
}
function setSubscribeNow(subscribeNow) {
return {
type: types.SET_SUBSCRIBE_NOW,
payload: subscribeNow,
};
}
export default {
composesGet,
composeStart,
composeUpdated,
composeGetStatus,
setRelease,
setUploadDestinations,
setUploadAWS,
setUploadAzure,
setUploadGoogle,
setSelectedPackages,
setSubscription,
setSubscribeNow,
};
export default { fetchComposes, fetchComposeStatus };

View file

@ -1 +0,0 @@
export { default as actions } from './actions';

View file

@ -0,0 +1,37 @@
import { createSlice } from '@reduxjs/toolkit';
const initialState = {
count: 0,
allIds: [],
byId: {},
error: null,
};
const composesSlice = createSlice({
name: 'composes',
initialState,
reducers: {
composeAdded: (state, action) => {
// only add to array if compose does not exist
if (!state.allIds.includes(action.payload.compose.id)) {
if (action.payload.insert) {
state.allIds.unshift(action.payload.compose.id);
} else {
state.allIds.push(action.payload.compose.id);
}
}
state.byId[action.payload.compose.id] = action.payload.compose;
state.error = null;
},
composesUpdatedCount: (state, action) => {
state.count = action.payload.count;
},
composeUpdatedStatus: (state, action) => {
state.byId[action.payload.id].image_status = action.payload.status;
},
},
});
export const { composeAdded, composesUpdatedCount, composeUpdatedStatus } =
composesSlice.actions;
export default composesSlice.reducer;

View file

@ -1,10 +1,10 @@
import { configureStore } from '@reduxjs/toolkit';
import promiseMiddleware from 'redux-promise-middleware';
import { notificationsReducer } from '@redhat-cloud-services/frontend-components-notifications/redux';
import composes from './reducers/composes';
import composesSlice from './composesSlice';
export const reducer = {
composes: composes,
composes: composesSlice,
notifications: notificationsReducer,
};

View file

@ -1,102 +0,0 @@
import types from '../types';
// Example of action.compose
// {
// "77e4c693-0497-4b85-936d-b2a3ad69571b": {
// created_at: "2021-04-21 11:20:46.927594 +0000 UTC",
// id: "77e4c693-0497-4b85-936d-b2a3ad69571b",
// request: {
// distribution: "rhel-8",
// image_requests: [
// {
// architecture: "x86_64",
// image_type: "ami",
// upload_request: {
// type: "aws",
// options: {}
// }
// }
// ]
// },
// image_status: {
// status: "uploading",
// },
// }
// };
const initialComposesState = {
count: 0,
allIds: [],
byId: {},
error: null,
};
// only add to array if compose does not exist
const updateAllIds = (allIds, id, insert) => {
if (allIds.includes(id)) {
return allIds;
}
if (insert) {
return [id].concat(allIds);
}
return allIds.concat(id);
};
export function composes(state = initialComposesState, action) {
switch (action.type) {
case types.COMPOSE_ADDED:
return {
...state,
allIds: updateAllIds(
state.allIds,
action.payload.compose.id,
action.payload.insert
),
byId: {
...state.byId,
[action.payload.compose.id]: action.payload.compose,
},
error: null,
};
case types.COMPOSE_FAILED:
return {
...state,
error: action.payload.error,
};
case types.COMPOSE_PENDING:
return {
...state,
error: null,
};
case types.COMPOSE_UPDATED:
return {
...state,
byId: {
...state.byId,
[action.payload.compose.id]: action.payload.compose,
},
};
case types.COMPOSES_UPDATED_COUNT:
return {
...state,
count: action.payload.count,
};
case types.COMPOSE_UPDATED_STATUS:
return {
...state,
byId: {
...state.byId,
[action.payload.id]: {
...state.byId[action.payload.id],
image_status: action.payload.status,
},
},
};
default:
return state;
}
}
export default composes;

View file

@ -1,29 +0,0 @@
const COMPOSE_ADDED = 'COMPOSE_ADDED';
const COMPOSE_FAILED = 'COMPOSE_FAILED';
const COMPOSE_UPDATED = 'COMPOSE_UPDATED';
const COMPOSES_UPDATED_COUNT = 'COMPOSES_UPDATED_COUNT';
const COMPOSE_UPDATED_STATUS = 'COMPOSE_UPDATED_STATUS';
const SET_RELEASE = 'SET_RELEASE';
const SET_UPLOAD_DESTINATIONS = 'SET_UPLOAD_DESTINATIONS';
const SET_UPLOAD_AWS = 'SET_UPLOAD_AWS';
const SET_UPLOAD_AZURE = 'SET_UPLOAD_AZURE';
const SET_UPLOAD_GOOGLE = 'SET_UPLOAD_GOOGLE';
const SET_SELECTED_PACKAGES = 'SET_SELECTED_PACKAGES';
const SET_SUBSCRIPTION = 'SET_SUBSCRIPTION';
const SET_SUBSCRIBE_NOW = 'SET_SUBSCRIBE_NOW';
export default {
COMPOSE_ADDED,
COMPOSE_FAILED,
COMPOSE_UPDATED,
COMPOSES_UPDATED_COUNT,
COMPOSE_UPDATED_STATUS,
SET_RELEASE,
SET_UPLOAD_DESTINATIONS,
SET_UPLOAD_AWS,
SET_UPLOAD_AZURE,
SET_UPLOAD_GOOGLE,
SET_SELECTED_PACKAGES,
SET_SUBSCRIPTION,
SET_SUBSCRIBE_NOW,
};

View file

@ -6,8 +6,8 @@ import api from '../../../api.js';
jest.mock('../../../store/actions/actions', () => {
return {
composesGet: () => ({ type: 'foo' }),
composeGetStatus: () => ({ type: 'bar' }),
fetchComposes: () => ({ type: 'foo' }),
fetchComposeStatus: () => ({ type: 'bar' }),
};
});

View file

@ -1,34 +0,0 @@
import { actions } from '../../store/actions';
import types from '../../store/types';
import { RHEL_8 } from '../../constants.js';
const compose = {
'77e4c693-0497-4b85-936d-b2a3ad69571b': {
id: '77e4c693-0497-4b85-936d-b2a3ad69571b',
distribution: RHEL_8,
image_requests: [
{
architecture: 'x86_64',
image_type: 'ami',
upload_request: {
type: 'aws',
options: {},
},
},
],
image_status: {
status: 'uploading',
},
},
};
describe('composeUpdated', () => {
test('returns dict', () => {
const result = actions.composeUpdated(compose);
// this function updates the type attribute and
// returns everything else unchanged
expect(result.type).toBe(types.COMPOSE_UPDATED);
expect(result.payload.compose).toBe(compose);
});
});

View file

@ -1,5 +1,4 @@
import { composes } from '../../store/reducers/composes';
import types from '../../store/types';
import composesSlice from '../../store/composesSlice';
import { RHEL_8 } from '../../constants.js';
const compose = {
@ -22,7 +21,7 @@ const compose = {
describe('composes', () => {
test('returns state for unknown actions', () => {
const result = composes(
const result = composesSlice(
{},
{
type: 'THIS-IS-UNKNOWN',
@ -32,15 +31,15 @@ describe('composes', () => {
expect(result).toEqual({});
});
test('returns updated state for types.COMPOSE_ADDED', () => {
test('returns updated state for composes/composeAdded', () => {
const state = {
allIds: [],
byId: {},
count: 1,
errors: null,
};
const result = composes(state, {
type: types.COMPOSE_ADDED,
const result = composesSlice(state, {
type: 'composes/composeAdded',
payload: { compose },
});
@ -52,44 +51,7 @@ describe('composes', () => {
expect(result.error).toEqual(null);
});
test('returns updated state for types.COMPOSE_UPDATED', () => {
const state = {
allIds: ['77e4c693-0497-4b85-936d-b2a3ad69571b'],
byId: {
'77e4c693-0497-4b85-936d-b2a3ad69571b': {},
},
count: 2,
error: null,
};
const result = composes(state, {
type: types.COMPOSE_UPDATED,
payload: { compose },
});
expect(result.allIds).toEqual(['77e4c693-0497-4b85-936d-b2a3ad69571b']);
expect(result.byId['77e4c693-0497-4b85-936d-b2a3ad69571b']).toEqual(
compose
);
expect(result.count).toEqual(2);
expect(result.error).toEqual(null);
});
test('returns updated state for types.COMPOSE_FAILED', () => {
const state = {
allIds: [],
byId: {},
count: 0,
error: null,
};
const result = composes(state, {
type: types.COMPOSE_FAILED,
payload: { error: 'test error' },
});
expect(result.error).toEqual('test error');
});
test('returns updated state for types.COMPOSES_UPDATED_COUNT', () => {
test('returns updated state for composes/composesUpdatedCount', () => {
const state = {
allIds: [],
byId: {},
@ -97,8 +59,8 @@ describe('composes', () => {
error: null,
};
const result = composes(state, {
type: types.COMPOSES_UPDATED_COUNT,
const result = composesSlice(state, {
type: 'composes/composesUpdatedCount',
payload: { count: 1 },
});