Skip to content

Commit

Permalink
refactor: Add TypeScript types for the Editors' Redux state (#1537)
Browse files Browse the repository at this point in the history
It starts to add type information to the messy Redux state used by the editors, mostly focusing on the state shared by all editors and the problem editor.
  • Loading branch information
bradenmacdonald authored Dec 3, 2024
1 parent 0771923 commit e75ce15
Show file tree
Hide file tree
Showing 18 changed files with 287 additions and 140 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ export const ToleranceTypes = {
type: 'None',
message: messages.typesNone,
},
};
} as const;
2 changes: 1 addition & 1 deletion src/editors/containers/ProblemEditor/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('ProblemEditor', () => {
});

describe('mapStateToProps', () => {
const testState = { A: 'pple', B: 'anana', C: 'ucumber' };
const testState = { A: 'pple', B: 'anana', C: 'ucumber' } as any;
test('blockValue from app.blockValue', () => {
expect(
mapStateToProps(testState).blockValue,
Expand Down
2 changes: 1 addition & 1 deletion src/editors/containers/ProblemEditor/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export interface Props extends EditorComponent {
/** null if this is a new problem */
problemType: ProblemType | null;
initializeProblemEditor: (blockValue: any) => void;
blockValue: Record<string, any>;
blockValue: Record<string, any> | null;
}

const ProblemEditor: React.FC<Props> = ({
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { createSlice } from '@reduxjs/toolkit';

import type { EditorState } from '..';
import { StrictDict } from '../../../utils';

const initialState = {
const initialState: EditorState['app'] = {
blockValue: null,
unitUrl: null,
blockContent: null,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
// import * in order to mock in-file references
import type { EditorState } from '..';
import { keyStore } from '../../../utils';
// import * in order to mock in-file references
import * as urls from '../../services/cms/urls';
import * as selectors from './selectors';

jest.mock('reselect', () => ({
createSelector: jest.fn((preSelectors, cb) => ({ preSelectors, cb })),
}));
jest.mock('../../services/cms/urls', () => ({
returnUrl: (args) => ({ returnUrl: args }),
}));
Expand All @@ -20,14 +18,14 @@ describe('app selectors unit tests', () => {
} = selectors;
describe('appSelector', () => {
it('returns the app data', () => {
expect(appSelector({ ...testState, app: testValue })).toEqual(testValue);
expect(appSelector({ ...testState, app: testValue } as any as EditorState)).toEqual(testValue);
});
});
describe('simpleSelectors', () => {
const testSimpleSelector = (key) => {
test(`${key} simpleSelector returns its value from the app store`, () => {
const { preSelectors, cb } = simpleSelectors[key];
expect(preSelectors).toEqual([appSelector]);
const { dependencies, resultFunc: cb } = simpleSelectors[key];
expect(dependencies).toEqual([appSelector]);
expect(cb({ ...testState, [key]: testValue })).toEqual(testValue);
});
};
Expand Down Expand Up @@ -55,17 +53,24 @@ describe('app selectors unit tests', () => {
});
describe('returnUrl', () => {
it('is memoized based on unitUrl and studioEndpointUrl', () => {
expect(selectors.returnUrl.preSelectors).toEqual([
expect(selectors.returnUrl.dependencies).toEqual([
simpleSelectors.unitUrl,
simpleSelectors.studioEndpointUrl,
simpleSelectors.learningContextId,
simpleSelectors.blockId,
]);
});
it('returns urls.returnUrl with the returnUrl', () => {
const { cb } = selectors.returnUrl;
const { resultFunc: cb } = selectors.returnUrl;
const studioEndpointUrl = 'baseURL';
const unitUrl = 'some unit url';
const unitUrl = {
data: {
ancestors: [
{
id: 'unit id', display_name: 'Unit', category: 'vertical' as const, has_children: true,
}],
},
};
const learningContextId = 'some learning context';
const blockId = 'block-v1 some v1 block id';
expect(
Expand All @@ -79,103 +84,103 @@ describe('app selectors unit tests', () => {
});
describe('isInitialized selector', () => {
it('is memoized based on editorInitialized, unitUrl, isLibrary and blockValue', () => {
expect(selectors.isInitialized.preSelectors).toEqual([
expect(selectors.isInitialized.dependencies).toEqual([
simpleSelectors.unitUrl,
simpleSelectors.blockValue,
selectors.isLibrary,
]);
});
describe('for library blocks', () => {
it('returns true if blockValue, and editorInitialized are truthy', () => {
const { cb } = selectors.isInitialized;
const { resultFunc: cb } = selectors.isInitialized;
const truthy = {
blockValue: { block: 'value' },
};

[
[[null, truthy.blockValue, true], true],
[[null, null, true], false],
[[null, truthy.blockValue, true] as [any, any, any], true] as const,
[[null, null, true] as [any, any, any], false] as const,
].map(([args, expected]) => expect(cb(...args)).toEqual(expected));
});
});
describe('for course blocks', () => {
it('returns true if blockValue, unitUrl, and editorInitialized are truthy', () => {
const { cb } = selectors.isInitialized;
const { resultFunc: cb } = selectors.isInitialized;
const truthy = {
blockValue: { block: 'value' },
unitUrl: { url: 'data' },
};

[
[[null, truthy.blockValue, false], false],
[[truthy.unitUrl, null, false], false],
[[truthy.unitUrl, truthy.blockValue, false], true],
[[null, truthy.blockValue, false] as [any, any, any], false] as const,
[[truthy.unitUrl, null, false] as [any, any, any], false] as const,
[[truthy.unitUrl, truthy.blockValue, false] as [any, any, any], true] as const,
].map(([args, expected]) => expect(cb(...args)).toEqual(expected));
});
});
});
describe('displayTitle', () => {
const title = 'tItLe';
it('is memoized based on blockType and blockTitle', () => {
expect(selectors.displayTitle.preSelectors).toEqual([
expect(selectors.displayTitle.dependencies).toEqual([
simpleSelectors.blockType,
simpleSelectors.blockTitle,
]);
});
it('returns null if blockType is null', () => {
expect(selectors.displayTitle.cb(null, title)).toEqual(null);
expect(selectors.displayTitle.resultFunc(null, title)).toEqual(null);
});
it('returns blockTitle if blockTitle is not null', () => {
expect(selectors.displayTitle.cb('html', title)).toEqual(title);
expect(selectors.displayTitle.resultFunc('html', title)).toEqual(title);
});
it('returns Text if the blockType is html', () => {
expect(selectors.displayTitle.cb('html', null)).toEqual('Text');
expect(selectors.displayTitle.resultFunc('html', null)).toEqual('Text');
});
it('returns the blockType capitalized if not html', () => {
expect(selectors.displayTitle.cb('video', null)).toEqual('Video');
expect(selectors.displayTitle.cb('random', null)).toEqual('Random');
expect(selectors.displayTitle.resultFunc('video', null)).toEqual('Video');
expect(selectors.displayTitle.resultFunc('random', null)).toEqual('Random');
});
});

describe('isLibrary', () => {
const learningContextIdLibrary = 'library-v1:name';
const learningContextIdCourse = 'course-v1:name';
it('is memoized based on isLibrary', () => {
expect(selectors.isLibrary.preSelectors).toEqual([
expect(selectors.isLibrary.dependencies).toEqual([
simpleSelectors.learningContextId,
simpleSelectors.blockId,
]);
});
describe('blockId is null', () => {
it('should return false when learningContextId null', () => {
expect(selectors.isLibrary.cb(null, null)).toEqual(false);
expect(selectors.isLibrary.resultFunc(null, null)).toEqual(false);
});
it('should return false when learningContextId defined', () => {
expect(selectors.isLibrary.cb(learningContextIdCourse, null)).toEqual(false);
expect(selectors.isLibrary.resultFunc(learningContextIdCourse, null)).toEqual(false);
});
});
describe('blockId is a course block', () => {
it('should return false when learningContextId null', () => {
expect(selectors.isLibrary.cb(null, 'block-v1:')).toEqual(false);
expect(selectors.isLibrary.resultFunc(null, 'block-v1:')).toEqual(false);
});
it('should return false when learningContextId defined', () => {
expect(selectors.isLibrary.cb(learningContextIdCourse, 'block-v1:')).toEqual(false);
expect(selectors.isLibrary.resultFunc(learningContextIdCourse, 'block-v1:')).toEqual(false);
});
});
describe('blockId is a v2 library block', () => {
it('should return true when learningContextId null', () => {
expect(selectors.isLibrary.cb(null, 'lb:')).toEqual(true);
expect(selectors.isLibrary.resultFunc(null, 'lb:')).toEqual(true);
});
it('should return false when learningContextId is a v1 library', () => {
expect(selectors.isLibrary.cb(learningContextIdLibrary, 'lb:')).toEqual(true);
expect(selectors.isLibrary.resultFunc(learningContextIdLibrary, 'lb:')).toEqual(true);
});
});
describe('blockId is a v1 library block', () => {
it('should return false when learningContextId null', () => {
expect(selectors.isLibrary.cb(null, 'library-v1')).toEqual(false);
expect(selectors.isLibrary.resultFunc(null, 'library-v1')).toEqual(false);
});
it('should return true when learningContextId a v1 library', () => {
expect(selectors.isLibrary.cb(learningContextIdLibrary, 'library-v1')).toEqual(true);
expect(selectors.isLibrary.resultFunc(learningContextIdLibrary, 'library-v1')).toEqual(true);
});
});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import { createSelector } from 'reselect';
import type { EditorState } from '..';
import { blockTypes } from '../../constants/app';
import { isLibraryV1Key } from '../../../../generic/key-utils';
import * as urls from '../../services/cms/urls';
// This 'module' self-import hack enables mocking during tests.
// See src/editors/decisions/0005-internal-editor-testability-decisions.md. The whole approach to how hooks are tested
// should be re-thought and cleaned up to avoid this pattern.
// eslint-disable-next-line import/no-self-import
import * as module from './selectors';

export const appSelector = (state) => state.app;
export const appSelector = (state: EditorState) => state.app;

const mkSimpleSelector = (cb) => createSelector([module.appSelector], cb);
const mkSimpleSelector = <T>(cb: (appState: EditorState['app']) => T) => createSelector([appSelector], cb);

// top-level app data selectors
export const simpleSelectors = {
Expand All @@ -19,6 +15,7 @@ export const simpleSelectors = {
blockType: mkSimpleSelector(app => app.blockType),
blockValue: mkSimpleSelector(app => app.blockValue),
studioView: mkSimpleSelector(app => app.studioView),
/** @deprecated Get as `const { learningContextid } = useEditorContext()` instead */
learningContextId: mkSimpleSelector(app => app.learningContextId),
editorInitialized: mkSimpleSelector(app => app.editorInitialized),
saveResponse: mkSimpleSelector(app => app.saveResponse),
Expand All @@ -32,8 +29,8 @@ export const simpleSelectors = {
};

export const returnUrl = createSelector(
[module.simpleSelectors.unitUrl, module.simpleSelectors.studioEndpointUrl, module.simpleSelectors.learningContextId,
module.simpleSelectors.blockId],
[simpleSelectors.unitUrl, simpleSelectors.studioEndpointUrl, simpleSelectors.learningContextId,
simpleSelectors.blockId],
(unitUrl, studioEndpointUrl, learningContextId, blockId) => (
urls.returnUrl({
studioEndpointUrl, unitUrl, learningContextId, blockId,
Expand All @@ -43,8 +40,8 @@ export const returnUrl = createSelector(

export const isLibrary = createSelector(
[
module.simpleSelectors.learningContextId,
module.simpleSelectors.blockId,
simpleSelectors.learningContextId,
simpleSelectors.blockId,
],
(learningContextId, blockId) => {
if (isLibraryV1Key(learningContextId)) {
Expand All @@ -59,9 +56,9 @@ export const isLibrary = createSelector(

export const isInitialized = createSelector(
[
module.simpleSelectors.unitUrl,
module.simpleSelectors.blockValue,
module.isLibrary,
simpleSelectors.unitUrl,
simpleSelectors.blockValue,
isLibrary,
],
(unitUrl, blockValue, isLibraryBlock) => {
if (isLibraryBlock) {
Expand All @@ -74,8 +71,8 @@ export const isInitialized = createSelector(

export const displayTitle = createSelector(
[
module.simpleSelectors.blockType,
module.simpleSelectors.blockTitle,
simpleSelectors.blockType,
simpleSelectors.blockTitle,
],
(blockType, blockTitle) => {
if (blockType === null) {
Expand All @@ -92,9 +89,9 @@ export const displayTitle = createSelector(

export const analytics = createSelector(
[
module.simpleSelectors.blockId,
module.simpleSelectors.blockType,
module.simpleSelectors.learningContextId,
simpleSelectors.blockId,
simpleSelectors.blockType,
simpleSelectors.learningContextId,
],
(blockId, blockType, learningContextId) => (
{ blockId, blockType, learningContextId }
Expand Down
34 changes: 0 additions & 34 deletions src/editors/data/redux/index.js

This file was deleted.

Loading

0 comments on commit e75ce15

Please sign in to comment.