From e32ffdf587a7a75f6116da990209a648f4ad01fe Mon Sep 17 00:00:00 2001 From: Vladislav Shkodin Date: Tue, 6 Apr 2021 19:28:15 +0300 Subject: [PATCH] refactor(core): refactor search actions and reducer Convert to TS Proper search action type Replace immutable with immer General cleanup --- .../src/actions/__tests__/search.spec.js | 22 +-- .../netlify-cms-core/src/actions/search.ts | 135 +++++++----------- .../Collection/Entries/EntriesSearch.js | 4 +- .../Editor/EditorControlPane/EditorControl.js | 8 +- .../netlify-cms-core/src/reducers/index.ts | 11 +- .../netlify-cms-core/src/reducers/search.js | 89 ------------ .../netlify-cms-core/src/reducers/search.ts | 88 ++++++++++++ packages/netlify-cms-core/src/types/redux.ts | 14 +- .../src/RelationControl.js | 10 +- .../src/__tests__/relation.spec.js | 9 +- 10 files changed, 162 insertions(+), 228 deletions(-) delete mode 100644 packages/netlify-cms-core/src/reducers/search.js create mode 100644 packages/netlify-cms-core/src/reducers/search.ts diff --git a/packages/netlify-cms-core/src/actions/__tests__/search.spec.js b/packages/netlify-cms-core/src/actions/__tests__/search.spec.js index 55f022f9..3852afae 100644 --- a/packages/netlify-cms-core/src/actions/__tests__/search.spec.js +++ b/packages/netlify-cms-core/src/actions/__tests__/search.spec.js @@ -22,7 +22,7 @@ describe('search', () => { it('should search entries in all collections using integration', async () => { const store = mockStore({ collections: fromJS({ posts: { name: 'posts' }, pages: { name: 'pages' } }), - search: fromJS({}), + search: {}, }); selectIntegration.mockReturnValue('search_integration'); @@ -46,8 +46,6 @@ describe('search', () => { expect(actions[1]).toEqual({ type: 'SEARCH_ENTRIES_SUCCESS', payload: { - searchTerm: 'find me', - searchCollections: ['posts', 'pages'], entries: response.entries, page: response.pagination, }, @@ -60,7 +58,7 @@ describe('search', () => { it('should search entries in a subset of collections using integration', async () => { const store = mockStore({ collections: fromJS({ posts: { name: 'posts' }, pages: { name: 'pages' } }), - search: fromJS({}), + search: {}, }); selectIntegration.mockReturnValue('search_integration'); @@ -84,8 +82,6 @@ describe('search', () => { expect(actions[1]).toEqual({ type: 'SEARCH_ENTRIES_SUCCESS', payload: { - searchTerm: 'find me', - searchCollections: ['pages'], entries: response.entries, page: response.pagination, }, @@ -98,7 +94,7 @@ describe('search', () => { it('should search entries in all collections using backend', async () => { const store = mockStore({ collections: fromJS({ posts: { name: 'posts' }, pages: { name: 'pages' } }), - search: fromJS({}), + search: {}, }); const response = { entries: [{ name: '1' }, { name: '' }], pagination: 1 }; @@ -121,8 +117,6 @@ describe('search', () => { expect(actions[1]).toEqual({ type: 'SEARCH_ENTRIES_SUCCESS', payload: { - searchTerm: 'find me', - searchCollections: ['posts', 'pages'], entries: response.entries, page: response.pagination, }, @@ -138,7 +132,7 @@ describe('search', () => { it('should search entries in a subset of collections using backend', async () => { const store = mockStore({ collections: fromJS({ posts: { name: 'posts' }, pages: { name: 'pages' } }), - search: fromJS({}), + search: {}, }); const response = { entries: [{ name: '1' }, { name: '' }], pagination: 1 }; @@ -161,8 +155,6 @@ describe('search', () => { expect(actions[1]).toEqual({ type: 'SEARCH_ENTRIES_SUCCESS', payload: { - searchTerm: 'find me', - searchCollections: ['pages'], entries: response.entries, page: response.pagination, }, @@ -175,7 +167,7 @@ describe('search', () => { it('should ignore identical search in all collections', async () => { const store = mockStore({ collections: fromJS({ posts: { name: 'posts' }, pages: { name: 'pages' } }), - search: fromJS({ isFetching: true, term: 'find me', collections: ['posts', 'pages'] }), + search: { isFetching: true, term: 'find me', collections: ['posts', 'pages'] }, }); await store.dispatch(searchEntries('find me')); @@ -187,7 +179,7 @@ describe('search', () => { it('should ignore identical search in a subset of collections', async () => { const store = mockStore({ collections: fromJS({ posts: { name: 'posts' }, pages: { name: 'pages' } }), - search: fromJS({ isFetching: true, term: 'find me', collections: ['pages'] }), + search: { isFetching: true, term: 'find me', collections: ['pages'] }, }); await store.dispatch(searchEntries('find me', ['pages'])); @@ -199,7 +191,7 @@ describe('search', () => { it('should not ignore same search term in different search collections', async () => { const store = mockStore({ collections: fromJS({ posts: { name: 'posts' }, pages: { name: 'pages' } }), - search: fromJS({ isFetching: true, term: 'find me', collections: ['pages'] }), + search: { isFetching: true, term: 'find me', collections: ['pages'] }, }); const backend = { search: jest.fn().mockResolvedValue({}) }; currentBackend.mockReturnValue(backend); diff --git a/packages/netlify-cms-core/src/actions/search.ts b/packages/netlify-cms-core/src/actions/search.ts index 25db3eac..fae87579 100644 --- a/packages/netlify-cms-core/src/actions/search.ts +++ b/packages/netlify-cms-core/src/actions/search.ts @@ -1,11 +1,11 @@ import { ThunkDispatch } from 'redux-thunk'; import { AnyAction } from 'redux'; +import { isEqual } from 'lodash'; import { State } from '../types/redux'; import { currentBackend } from '../backend'; import { getIntegrationProvider } from '../integrations'; import { selectIntegration } from '../reducers'; import { EntryValue } from '../valueObjects/Entry'; -import { List } from 'immutable'; /* * Constant Declarations @@ -14,9 +14,9 @@ export const SEARCH_ENTRIES_REQUEST = 'SEARCH_ENTRIES_REQUEST'; export const SEARCH_ENTRIES_SUCCESS = 'SEARCH_ENTRIES_SUCCESS'; export const SEARCH_ENTRIES_FAILURE = 'SEARCH_ENTRIES_FAILURE'; -export const QUERY_REQUEST = 'INIT_QUERY'; -export const QUERY_SUCCESS = 'QUERY_OK'; -export const QUERY_FAILURE = 'QUERY_ERROR'; +export const QUERY_REQUEST = 'QUERY_REQUEST'; +export const QUERY_SUCCESS = 'QUERY_SUCCESS'; +export const QUERY_FAILURE = 'QUERY_FAILURE'; export const SEARCH_CLEAR = 'SEARCH_CLEAR'; @@ -28,51 +28,33 @@ export function searchingEntries(searchTerm: string, searchCollections: string[] return { type: SEARCH_ENTRIES_REQUEST, payload: { searchTerm, searchCollections, page }, - }; + } as const; } -export function searchSuccess( - searchTerm: string, - searchCollections: string[], - entries: EntryValue[], - page: number, -) { +export function searchSuccess(entries: EntryValue[], page: number) { return { type: SEARCH_ENTRIES_SUCCESS, payload: { - searchTerm, - searchCollections, entries, page, }, - }; + } as const; } -export function searchFailure(searchTerm: string, error: Error) { +export function searchFailure(error: Error) { return { type: SEARCH_ENTRIES_FAILURE, - payload: { - searchTerm, - error, - }, - }; + payload: { error }, + } as const; } -export function querying( - namespace: string, - collection: string, - searchFields: string[], - searchTerm: string, -) { +export function querying(searchTerm: string) { return { type: QUERY_REQUEST, payload: { - namespace, - collection, - searchFields, searchTerm, }, - }; + } as const; } type SearchResponse = { @@ -85,42 +67,21 @@ type QueryResponse = { query: string; }; -export function querySuccess( - namespace: string, - collection: string, - searchFields: string[], - searchTerm: string, - response: QueryResponse, -) { +export function querySuccess(namespace: string, hits: EntryValue[]) { return { type: QUERY_SUCCESS, payload: { namespace, - collection, - searchFields, - searchTerm, - response, + hits, }, - }; + } as const; } -export function queryFailure( - namespace: string, - collection: string, - searchFields: string[], - searchTerm: string, - error: Error, -) { +export function queryFailure(error: Error) { return { type: QUERY_FAILURE, - payload: { - namespace, - collection, - searchFields, - searchTerm, - error, - }, - }; + payload: { error }, + } as const; } /* @@ -128,7 +89,7 @@ export function queryFailure( */ export function clearSearch() { - return { type: SEARCH_CLEAR }; + return { type: SEARCH_CLEAR } as const; } /* @@ -136,33 +97,29 @@ export function clearSearch() { */ // SearchEntries will search for complete entries in all collections. -export function searchEntries( - searchTerm: string, - searchCollections: string[] | null = null, - page = 0, -) { - return (dispatch: ThunkDispatch, getState: () => State) => { +export function searchEntries(searchTerm: string, searchCollections: string[], page = 0) { + return async (dispatch: ThunkDispatch, getState: () => State) => { const state = getState(); const { search } = state; const backend = currentBackend(state.config); const allCollections = searchCollections || state.collections.keySeq().toArray(); const collections = allCollections.filter(collection => - selectIntegration(state, collection as string, 'search'), + selectIntegration(state, collection, 'search'), ); - const integration = selectIntegration(state, collections[0] as string, 'search'); + const integration = selectIntegration(state, collections[0], 'search'); // avoid duplicate searches if ( - search.get('isFetching') === true && - search.get('term') === searchTerm && - search.get('collections') !== null && - List(allCollections).equals(search.get('collections') as List) && + search.isFetching && + search.term === searchTerm && + isEqual(allCollections, search.collections) && // if an integration doesn't exist, 'page' is not used - (search.get('page') === page || !integration) + (search.page === page || !integration) ) { return; } - dispatch(searchingEntries(searchTerm, allCollections as string[], page)); + + dispatch(searchingEntries(searchTerm, allCollections, page)); const searchPromise = integration ? getIntegrationProvider(state.integrations, backend.getToken, integration).search( @@ -178,18 +135,12 @@ export function searchEntries( searchTerm, ); - return searchPromise.then( - (response: SearchResponse) => - dispatch( - searchSuccess( - searchTerm, - allCollections as string[], - response.entries, - response.pagination, - ), - ), - (error: Error) => dispatch(searchFailure(searchTerm, error)), - ); + try { + const response: SearchResponse = await searchPromise; + return dispatch(searchSuccess(response.entries, response.pagination)); + } catch (error) { + return dispatch(searchFailure(error)); + } }; } @@ -204,7 +155,7 @@ export function query( limit?: number, ) { return async (dispatch: ThunkDispatch, getState: () => State) => { - dispatch(querying(namespace, collectionName, searchFields, searchTerm)); + dispatch(querying(searchTerm)); const state = getState(); const backend = currentBackend(state.config); @@ -223,9 +174,19 @@ export function query( try { const response: QueryResponse = await queryPromise; - return dispatch(querySuccess(namespace, collectionName, searchFields, searchTerm, response)); + return dispatch(querySuccess(namespace, response.hits)); } catch (error) { - return dispatch(queryFailure(namespace, collectionName, searchFields, searchTerm, error)); + return dispatch(queryFailure(error)); } }; } + +export type SearchAction = ReturnType< + | typeof searchingEntries + | typeof searchSuccess + | typeof searchFailure + | typeof querying + | typeof querySuccess + | typeof queryFailure + | typeof clearSearch +>; diff --git a/packages/netlify-cms-core/src/components/Collection/Entries/EntriesSearch.js b/packages/netlify-cms-core/src/components/Collection/Entries/EntriesSearch.js index 57cf5363..57b9488e 100644 --- a/packages/netlify-cms-core/src/components/Collection/Entries/EntriesSearch.js +++ b/packages/netlify-cms-core/src/components/Collection/Entries/EntriesSearch.js @@ -76,8 +76,8 @@ function mapStateToProps(state, ownProps) { const { searchTerm } = ownProps; const collections = ownProps.collections.toIndexedSeq(); const collectionNames = ownProps.collections.keySeq().toArray(); - const isFetching = state.search.get('isFetching'); - const page = state.search.get('page'); + const isFetching = state.search.isFetching; + const page = state.search.page; const entries = selectSearchedEntries(state, collectionNames); return { isFetching, page, collections, collectionNames, entries, searchTerm }; } diff --git a/packages/netlify-cms-core/src/components/Editor/EditorControlPane/EditorControl.js b/packages/netlify-cms-core/src/components/Editor/EditorControlPane/EditorControl.js index 6075039b..57d2a411 100644 --- a/packages/netlify-cms-core/src/components/Editor/EditorControlPane/EditorControl.js +++ b/packages/netlify-cms-core/src/components/Editor/EditorControlPane/EditorControl.js @@ -129,7 +129,7 @@ class EditorControl extends React.Component { processControlRef: PropTypes.func, controlRef: PropTypes.func, query: PropTypes.func.isRequired, - queryHits: PropTypes.oneOfType([PropTypes.array, PropTypes.object]), + queryHits: PropTypes.object, isFetching: PropTypes.bool, clearSearch: PropTypes.func.isRequired, clearFieldErrors: PropTypes.func.isRequired, @@ -311,7 +311,7 @@ class EditorControl extends React.Component { editorControl={ConnectedEditorControl} query={query} loadEntry={loadEntry} - queryHits={queryHits} + queryHits={queryHits[this.uniqueFieldId] || []} clearSearch={clearSearch} clearFieldErrors={clearFieldErrors} isFetching={isFetching} @@ -356,8 +356,8 @@ function mapStateToProps(state) { return { mediaPaths: state.mediaLibrary.get('controlMedia'), - isFetching: state.search.get('isFetching'), - queryHits: state.search.get('queryHits'), + isFetching: state.search.isFetching, + queryHits: state.search.queryHits, config: state.config, entry, collection, diff --git a/packages/netlify-cms-core/src/reducers/index.ts b/packages/netlify-cms-core/src/reducers/index.ts index e5bbbe7a..ad23672e 100644 --- a/packages/netlify-cms-core/src/reducers/index.ts +++ b/packages/netlify-cms-core/src/reducers/index.ts @@ -1,3 +1,4 @@ +import { List } from 'immutable'; import auth from './auth'; import config from './config'; import integrations, * as fromIntegrations from './integrations'; @@ -50,14 +51,10 @@ export function selectPublishedSlugs(state: State, collection: string) { } export function selectSearchedEntries(state: State, availableCollections: string[]) { - const searchItems = state.search.get('entryIds'); // only return search results for actually available collections - return ( - searchItems && - searchItems - .filter(({ collection }) => availableCollections.indexOf(collection) !== -1) - .map(({ collection, slug }) => fromEntries.selectEntry(state.entries, collection, slug)) - ); + return List(state.search.entryIds) + .filter(entryId => availableCollections.indexOf(entryId!.collection) !== -1) + .map(entryId => fromEntries.selectEntry(state.entries, entryId!.collection, entryId!.slug)); } export function selectDeployPreview(state: State, collection: string, slug: string) { diff --git a/packages/netlify-cms-core/src/reducers/search.js b/packages/netlify-cms-core/src/reducers/search.js deleted file mode 100644 index 7e4bef6c..00000000 --- a/packages/netlify-cms-core/src/reducers/search.js +++ /dev/null @@ -1,89 +0,0 @@ -import { Map, List } from 'immutable'; - -import { - SEARCH_ENTRIES_REQUEST, - SEARCH_ENTRIES_SUCCESS, - QUERY_REQUEST, - QUERY_SUCCESS, - SEARCH_CLEAR, -} from 'Actions/search'; - -let loadedEntries; -let response; -let page; -let searchTerm; -let searchCollections; - -const defaultState = Map({ - isFetching: false, - term: null, - collections: null, - page: 0, - entryIds: List([]), - queryHits: Map({}), -}); - -function entries(state = defaultState, action) { - switch (action.type) { - case SEARCH_CLEAR: - return defaultState; - - case SEARCH_ENTRIES_REQUEST: - if (action.payload.searchTerm !== state.get('term')) { - return state.withMutations(map => { - map.set('isFetching', true); - map.set('term', action.payload.searchTerm); - map.set('collections', List(action.payload.searchCollections)); - map.set('page', action.payload.page); - }); - } - return state; - - case SEARCH_ENTRIES_SUCCESS: - loadedEntries = action.payload.entries; - page = action.payload.page; - searchTerm = action.payload.searchTerm; - searchCollections = action.payload.searchCollections; - return state.withMutations(map => { - const entryIds = List( - loadedEntries.map(entry => ({ collection: entry.collection, slug: entry.slug })), - ); - map.set('isFetching', false); - map.set('fetchID', null); - map.set('page', page); - map.set('term', searchTerm); - map.set('collections', List(searchCollections)); - map.set( - 'entryIds', - !page || isNaN(page) || page === 0 - ? entryIds - : map.get('entryIds', List()).concat(entryIds), - ); - }); - - case QUERY_REQUEST: - if (action.payload.searchTerm !== state.get('term')) { - return state.withMutations(map => { - map.set('isFetching', action.payload.namespace ? true : false); - map.set('fetchID', action.payload.namespace); - map.set('term', action.payload.searchTerm); - }); - } - return state; - - case QUERY_SUCCESS: - searchTerm = action.payload.searchTerm; - response = action.payload.response; - return state.withMutations(map => { - map.set('isFetching', false); - map.set('fetchID', null); - map.set('term', searchTerm); - map.mergeIn(['queryHits'], Map({ [action.payload.namespace]: response.hits })); - }); - - default: - return state; - } -} - -export default entries; diff --git a/packages/netlify-cms-core/src/reducers/search.ts b/packages/netlify-cms-core/src/reducers/search.ts new file mode 100644 index 00000000..a7e78f1b --- /dev/null +++ b/packages/netlify-cms-core/src/reducers/search.ts @@ -0,0 +1,88 @@ +import { produce } from 'immer'; + +import { + QUERY_FAILURE, + QUERY_REQUEST, + QUERY_SUCCESS, + SEARCH_CLEAR, + SEARCH_ENTRIES_FAILURE, + SEARCH_ENTRIES_REQUEST, + SEARCH_ENTRIES_SUCCESS, + SearchAction, +} from '../actions/search'; +import { EntryValue } from '../valueObjects/Entry'; + +export type Search = { + isFetching: boolean; + term: string; + collections: string[]; + page: number; + entryIds: { collection: string; slug: string }[]; + queryHits: Record; + error: Error | undefined; +}; + +const defaultState: Search = { + isFetching: false, + term: '', + collections: [], + page: 0, + entryIds: [], + queryHits: {}, + error: undefined, +}; + +const search = produce((state: Search, action: SearchAction) => { + switch (action.type) { + case SEARCH_CLEAR: + return defaultState; + + case SEARCH_ENTRIES_REQUEST: { + const { page, searchTerm, searchCollections } = action.payload; + state.isFetching = true; + state.term = searchTerm; + state.collections = searchCollections; + state.page = page; + break; + } + + case SEARCH_ENTRIES_SUCCESS: { + const { entries, page } = action.payload; + const entryIds = entries.map(entry => ({ collection: entry.collection, slug: entry.slug })); + state.isFetching = false; + state.page = page; + state.entryIds = + !page || isNaN(page) || page === 0 ? entryIds : state.entryIds.concat(entryIds); + break; + } + + case SEARCH_ENTRIES_FAILURE: { + const { error } = action.payload; + state.isFetching = false; + state.error = error; + break; + } + + case QUERY_REQUEST: { + const { searchTerm } = action.payload; + state.isFetching = true; + state.term = searchTerm; + break; + } + + case QUERY_SUCCESS: { + const { namespace, hits } = action.payload; + state.isFetching = false; + state.queryHits[namespace] = hits; + break; + } + + case QUERY_FAILURE: { + const { error } = action.payload; + state.isFetching = false; + state.error = error; + } + } +}, defaultState); + +export default search; diff --git a/packages/netlify-cms-core/src/types/redux.ts b/packages/netlify-cms-core/src/types/redux.ts index c3da40f5..d1ac97fc 100644 --- a/packages/netlify-cms-core/src/types/redux.ts +++ b/packages/netlify-cms-core/src/types/redux.ts @@ -7,6 +7,7 @@ import { Auth } from '../reducers/auth'; import { Status } from '../reducers/status'; import { Medias } from '../reducers/medias'; import { Deploys } from '../reducers/deploys'; +import { Search } from '../reducers/search'; export type CmsBackendType = | 'azure' @@ -675,19 +676,6 @@ export type Integrations = StaticallyTypedRecord<{ hooks: { [collectionOrHook: string]: any }; }>; -interface SearchItem { - collection: string; - slug: string; -} - -export type Search = StaticallyTypedRecord<{ - entryIds?: SearchItem[]; - isFetching: boolean; - term: string | null; - collections: List | null; - page: number; -}>; - export type Cursors = StaticallyTypedRecord<{}>; export interface State { diff --git a/packages/netlify-cms-widget-relation/src/RelationControl.js b/packages/netlify-cms-widget-relation/src/RelationControl.js index b910ca9f..7d537511 100644 --- a/packages/netlify-cms-widget-relation/src/RelationControl.js +++ b/packages/netlify-cms-widget-relation/src/RelationControl.js @@ -90,9 +90,8 @@ export default class RelationControl extends React.Component { forID: PropTypes.string.isRequired, value: PropTypes.node, field: ImmutablePropTypes.map, - fetchID: PropTypes.string, query: PropTypes.func.isRequired, - queryHits: PropTypes.oneOfType([PropTypes.array, PropTypes.object]), + queryHits: PropTypes.array, classNameWrapper: PropTypes.string.isRequired, setActiveStyle: PropTypes.func.isRequired, setInactiveStyle: PropTypes.func.isRequired, @@ -119,7 +118,7 @@ export default class RelationControl extends React.Component { const metadata = {}; const searchFieldsArray = getSearchFieldArray(field.get('search_fields')); const { payload } = await query(forID, collection, searchFieldsArray, '', file); - const hits = payload.response?.hits || []; + const hits = payload.hits || []; const options = this.parseHitOptions(hits); const initialOptions = initialSearchValues .map(v => { @@ -221,7 +220,7 @@ export default class RelationControl extends React.Component { const file = field.get('file'); query(forID, collection, searchFieldsArray, term, file, optionsLength).then(({ payload }) => { - const hits = payload.response?.hits || []; + const hits = payload.hits || []; const options = this.parseHitOptions(hits); const uniq = uniqOptions(this.state.initialOptions, options); callback(uniq); @@ -241,8 +240,7 @@ export default class RelationControl extends React.Component { const isMultiple = this.isMultiple(); const isClearable = !field.get('required', true) || isMultiple; - const hits = queryHits.get(forID, []); - const queryOptions = this.parseHitOptions(hits); + const queryOptions = this.parseHitOptions(queryHits); const options = uniqOptions(this.state.initialOptions, queryOptions); const selectedValue = getSelectedValue({ options, diff --git a/packages/netlify-cms-widget-relation/src/__tests__/relation.spec.js b/packages/netlify-cms-widget-relation/src/__tests__/relation.spec.js index f60e85ba..c2ad480e 100644 --- a/packages/netlify-cms-widget-relation/src/__tests__/relation.spec.js +++ b/packages/netlify-cms-widget-relation/src/__tests__/relation.spec.js @@ -1,5 +1,5 @@ import React from 'react'; -import { fromJS, Map } from 'immutable'; +import { fromJS } from 'immutable'; import { render, fireEvent, waitFor } from '@testing-library/react'; import { NetlifyCmsWidgetRelation } from '../'; @@ -137,7 +137,7 @@ const numberFieldsHits = [ class RelationController extends React.Component { state = { value: this.props.value, - queryHits: Map(), + queryHits: [], }; mounted = false; @@ -154,9 +154,8 @@ class RelationController extends React.Component { this.setState({ ...this.state, value }); }); - setQueryHits = jest.fn(hits => { + setQueryHits = jest.fn(queryHits => { if (this.mounted) { - const queryHits = Map().set('relation-field', hits); this.setState({ ...this.state, queryHits }); } }); @@ -186,7 +185,7 @@ class RelationController extends React.Component { this.setQueryHits(hits); - return Promise.resolve({ payload: { response: { hits } } }); + return Promise.resolve({ payload: { hits } }); }); render() {