From 9a4c317f20f361961cf3cdb38459d776f962e9b9 Mon Sep 17 00:00:00 2001 From: Hetu Nandu Date: Wed, 24 Mar 2021 10:39:47 +0530 Subject: [PATCH] Store value improvements (#3663) --- .github/workflows/client-build.yml | 2 +- app/client/src/actions/pageActions.tsx | 11 +- app/client/src/components/ads/Dropdown.tsx | 1 - app/client/src/constants/AppConstants.ts | 2 +- .../src/constants/ReduxActionConstants.tsx | 3 +- .../src/entities/DataTree/dataTreeFactory.ts | 9 +- .../src/reducers/entityReducers/appReducer.ts | 31 ++- app/client/src/sagas/ActionExecutionSagas.ts | 53 +++--- app/client/src/sagas/EvaluationsSaga.ts | 3 +- app/client/src/sagas/InitSagas.ts | 10 +- app/client/src/sagas/userSagas.tsx | 2 +- app/client/src/selectors/entitiesSelector.ts | 4 + app/client/src/utils/localStorage.test.ts | 29 +++ app/client/src/utils/localStorage.tsx | 9 + .../src/workers/evaluationUtils.test.ts | 178 +++++++++++++++++- app/client/src/workers/evaluationUtils.ts | 8 +- 16 files changed, 308 insertions(+), 47 deletions(-) create mode 100644 app/client/src/utils/localStorage.test.ts diff --git a/.github/workflows/client-build.yml b/.github/workflows/client-build.yml index 16f04c9dd4..6bdb9294f0 100644 --- a/.github/workflows/client-build.yml +++ b/.github/workflows/client-build.yml @@ -94,7 +94,7 @@ jobs: - name: Run the jest tests if: github.event_name == 'pull_request' - uses: hetunandu/Jest-Coverage-Diff@feature/improve-comment-ui + uses: hetunandu/Jest-Coverage-Diff@fix/new-delete-file with: fullCoverageDiff: false runCommand: cd app/client && REACT_APP_ENVIRONMENT=${{steps.vars.outputs.REACT_APP_ENVIRONMENT}} yarn run test:unit diff --git a/app/client/src/actions/pageActions.tsx b/app/client/src/actions/pageActions.tsx index 1d4db35c69..a199240e90 100644 --- a/app/client/src/actions/pageActions.tsx +++ b/app/client/src/actions/pageActions.tsx @@ -259,11 +259,18 @@ export const setAppMode = (payload: APP_MODE): ReduxAction => { }; }; -export const updateAppStore = ( +export const updateAppTransientStore = ( + payload: Record, +): ReduxAction> => ({ + type: ReduxActionTypes.UPDATE_APP_TRANSIENT_STORE, + payload, +}); + +export const updateAppPersistentStore = ( payload: Record, ): ReduxAction> => { return { - type: ReduxActionTypes.UPDATE_APP_STORE, + type: ReduxActionTypes.UPDATE_APP_PERSISTENT_STORE, payload, }; }; diff --git a/app/client/src/components/ads/Dropdown.tsx b/app/client/src/components/ads/Dropdown.tsx index 06d3eff9de..7a092bb2b1 100644 --- a/app/client/src/components/ads/Dropdown.tsx +++ b/app/client/src/components/ads/Dropdown.tsx @@ -223,7 +223,6 @@ export default function Dropdown(props: DropdownProps) { }, [onSelect], ); - console.log({ height: props.height }); return ( `${APP_STORE_NAMESPACE}-${appId}`; -export const getAppStore = (appId: string) => { +export const getPersistentAppStore = (appId: string) => { const appStoreName = getAppStoreName(appId); let storeString = "{}"; // Check if localStorage exists diff --git a/app/client/src/constants/ReduxActionConstants.tsx b/app/client/src/constants/ReduxActionConstants.tsx index 01bd577632..ab8a831d55 100644 --- a/app/client/src/constants/ReduxActionConstants.tsx +++ b/app/client/src/constants/ReduxActionConstants.tsx @@ -318,7 +318,8 @@ export const ReduxActionTypes: { [key: string]: string } = { SET_APP_MODE: "SET_APP_MODE", TOGGLE_PROPERTY_PANE_WIDGET_NAME_EDIT: "TOGGLE_PROPERTY_PANE_WIDGET_NAME_EDIT", - UPDATE_APP_STORE: "UPDATE_APP_STORE", + UPDATE_APP_PERSISTENT_STORE: "UPDATE_APP_PERSISTENT_STORE", + UPDATE_APP_TRANSIENT_STORE: "UPDATE_APP_TRANSIENT_STORE", SET_ACTION_TO_EXECUTE_ON_PAGELOAD: "SET_ACTION_TO_EXECUTE_ON_PAGELOAD", TOGGLE_ACTION_EXECUTE_ON_LOAD_SUCCESS: "TOGGLE_ACTION_EXECUTE_ON_LOAD_SUCCESS", diff --git a/app/client/src/entities/DataTree/dataTreeFactory.ts b/app/client/src/entities/DataTree/dataTreeFactory.ts index 612bb5f26d..773bf0e607 100644 --- a/app/client/src/entities/DataTree/dataTreeFactory.ts +++ b/app/client/src/entities/DataTree/dataTreeFactory.ts @@ -59,7 +59,7 @@ export interface DataTreeWidget extends WidgetProps { ENTITY_TYPE: ENTITY_TYPE.WIDGET; } -export interface DataTreeAppsmith extends AppDataState { +export interface DataTreeAppsmith extends Omit { ENTITY_TYPE: ENTITY_TYPE.APPSMITH; store: Record; } @@ -192,7 +192,12 @@ export class DataTreeFactory { }); dataTree.pageList = pageList; - dataTree.appsmith = { ...appData } as DataTreeAppsmith; + dataTree.appsmith = { + ...appData, + // combine both persistent and transient state with the transient state + // taking precedence in case the key is the same + store: { ...appData.store.persistent, ...appData.store.transient }, + } as DataTreeAppsmith; (dataTree.appsmith as DataTreeAppsmith).ENTITY_TYPE = ENTITY_TYPE.APPSMITH; return dataTree; } diff --git a/app/client/src/reducers/entityReducers/appReducer.ts b/app/client/src/reducers/entityReducers/appReducer.ts index 24de95d322..d11575ccd7 100644 --- a/app/client/src/reducers/entityReducers/appReducer.ts +++ b/app/client/src/reducers/entityReducers/appReducer.ts @@ -24,11 +24,16 @@ export type UrlDataState = { fullPath: string; }; +export type AppStoreState = { + transient: Record; + persistent: Record; +}; + export type AppDataState = { mode?: APP_MODE; user: AuthUserState; URL: UrlDataState; - store: Record; + store: AppStoreState; }; const initialState: AppDataState = { @@ -47,7 +52,10 @@ const initialState: AppDataState = { hash: "", fullPath: "", }, - store: {}, + store: { + transient: {}, + persistent: {}, + }, }; const appReducer = createReducer(initialState, { @@ -78,13 +86,28 @@ const appReducer = createReducer(initialState, { URL: action.payload, }; }, - [ReduxActionTypes.UPDATE_APP_STORE]: ( + [ReduxActionTypes.UPDATE_APP_TRANSIENT_STORE]: ( state: AppDataState, action: ReduxAction>, ) => { return { ...state, - store: action.payload, + store: { + ...state.store, + transient: action.payload, + }, + }; + }, + [ReduxActionTypes.UPDATE_APP_PERSISTENT_STORE]: ( + state: AppDataState, + action: ReduxAction>, + ) => { + return { + ...state, + store: { + ...state.store, + persistent: action.payload, + }, }; }, }); diff --git a/app/client/src/sagas/ActionExecutionSagas.ts b/app/client/src/sagas/ActionExecutionSagas.ts index 72da00f564..eb78ecea91 100644 --- a/app/client/src/sagas/ActionExecutionSagas.ts +++ b/app/client/src/sagas/ActionExecutionSagas.ts @@ -22,7 +22,6 @@ import { takeEvery, takeLatest, } from "redux-saga/effects"; -import { getDynamicBindings, isDynamicValue } from "utils/DynamicBindingUtils"; import { ActionDescription, RunActionPayload, @@ -57,6 +56,7 @@ import ActionAPI, { } from "api/ActionAPI"; import { getAction, + getAppStoreData, getCurrentPageNameByActionId, isActionDirty, isActionSaving, @@ -67,7 +67,10 @@ import { validateResponse } from "sagas/ErrorSagas"; import { TypeOptions } from "react-toastify"; import { PLUGIN_TYPE_API } from "constants/ApiEditorConstants"; import { DEFAULT_EXECUTE_ACTION_TIMEOUT_MS } from "constants/ApiConstants"; -import { updateAppStore } from "actions/pageActions"; +import { + updateAppPersistentStore, + updateAppTransientStore, +} from "actions/pageActions"; import { getAppStoreName } from "constants/AppConstants"; import downloadjs from "downloadjs"; import { getType, Types } from "utils/TypeHelpers"; @@ -94,7 +97,7 @@ import { ERROR_FAIL_ON_PAGE_LOAD_ACTIONS, ERROR_WIDGET_DOWNLOAD, } from "constants/messages"; -import { EMPTY_RESPONSE } from "../components/editorComponents/ApiResponseView"; +import { EMPTY_RESPONSE } from "components/editorComponents/ApiResponseView"; import localStorage from "utils/localStorage"; import { getWidgetByName } from "./selectors"; @@ -175,18 +178,32 @@ function* navigateActionSaga( } function* storeValueLocally( - action: { key: string; value: string }, + action: { key: string; value: string; persist: boolean }, event: ExecuteActionPayloadEvent, ) { try { - const appId = yield select(getCurrentApplicationId); - const appStoreName = getAppStoreName(appId); - const existingStore = yield localStorage.getItem(appStoreName) || "{}"; - const storeObj = JSON.parse(existingStore); - storeObj[action.key] = action.value; - const storeString = JSON.stringify(storeObj); - yield localStorage.setItem(appStoreName, storeString); - yield put(updateAppStore(storeObj)); + if (action.persist) { + const appId = yield select(getCurrentApplicationId); + const appStoreName = getAppStoreName(appId); + const existingStore = localStorage.getItem(appStoreName) || "{}"; + const parsedStore = JSON.parse(existingStore); + parsedStore[action.key] = action.value; + const storeString = JSON.stringify(parsedStore); + yield localStorage.setItem(appStoreName, storeString); + yield put(updateAppPersistentStore(parsedStore)); + } else { + const existingStore = yield select(getAppStoreData); + const newTransientStore = { + ...existingStore.transient, + [action.key]: action.value, + }; + yield put(updateAppTransientStore(newTransientStore)); + } + // Wait for an evaluation before completing this trigger effect + // This makes this trigger work in sync and not trigger + // another effect till the values are reflected in + // the dataTree + yield take(ReduxActionTypes.SET_EVALUATED_TREE); if (event.callback) event.callback({ success: true }); } catch (err) { if (event.callback) event.callback({ success: false }); @@ -389,18 +406,6 @@ export function* evaluateActionParams( return mapToPropList(actionParams); } -export function extractBindingsFromAction(action: Action) { - const bindings: string[] = []; - action.dynamicBindingPathList.forEach((a) => { - const value = _.get(action, a.key); - if (isDynamicValue(value)) { - const { jsSnippets } = getDynamicBindings(value); - bindings.push(...jsSnippets.filter((jsSnippet) => !!jsSnippet)); - } - }); - return bindings; -} - export function* executeActionSaga( apiAction: RunActionPayload, event: ExecuteActionPayloadEvent, diff --git a/app/client/src/sagas/EvaluationsSaga.ts b/app/client/src/sagas/EvaluationsSaga.ts index 691892b9db..e3bb2ed77d 100644 --- a/app/client/src/sagas/EvaluationsSaga.ts +++ b/app/client/src/sagas/EvaluationsSaga.ts @@ -246,7 +246,8 @@ const EVALUATE_REDUX_ACTIONS = [ // App Data ReduxActionTypes.SET_APP_MODE, ReduxActionTypes.FETCH_USER_DETAILS_SUCCESS, - ReduxActionTypes.UPDATE_APP_STORE, + ReduxActionTypes.UPDATE_APP_PERSISTENT_STORE, + ReduxActionTypes.UPDATE_APP_TRANSIENT_STORE, // Widgets ReduxActionTypes.UPDATE_LAYOUT, ReduxActionTypes.UPDATE_WIDGET_PROPERTY, diff --git a/app/client/src/sagas/InitSagas.ts b/app/client/src/sagas/InitSagas.ts index 7fd658d49d..b9e42ad991 100644 --- a/app/client/src/sagas/InitSagas.ts +++ b/app/client/src/sagas/InitSagas.ts @@ -22,7 +22,7 @@ import { fetchPageList, fetchPublishedPage, setAppMode, - updateAppStore, + updateAppPersistentStore, } from "actions/pageActions"; import { fetchDatasources } from "actions/datasourceActions"; import { fetchPlugins } from "actions/pluginActions"; @@ -31,7 +31,7 @@ import { fetchApplication } from "actions/applicationActions"; import AnalyticsUtil from "utils/AnalyticsUtil"; import { getCurrentApplication } from "selectors/applicationSelectors"; import { APP_MODE } from "reducers/entityReducers/appReducer"; -import { getAppStore } from "constants/AppConstants"; +import { getPersistentAppStore } from "constants/AppConstants"; import { getDefaultPageId } from "./selectors"; import { populatePageDSLsSaga } from "./PageSagas"; import log from "loglevel"; @@ -48,6 +48,7 @@ function* initializeEditorSaga( const { applicationId, pageId } = initializeEditorAction.payload; try { yield put(setAppMode(APP_MODE.EDIT)); + yield put(updateAppPersistentStore(getPersistentAppStore(applicationId))); yield put({ type: ReduxActionTypes.START_EVALUATION }); yield all([ put(fetchPageList(applicationId, APP_MODE.EDIT)), @@ -115,8 +116,6 @@ function* initializeEditorSaga( return; } - yield put(updateAppStore(getAppStore(applicationId))); - const currentApplication = yield select(getCurrentApplication); const appName = currentApplication ? currentApplication.name : ""; @@ -150,6 +149,7 @@ export function* initializeAppViewerSaga( ) { const { applicationId, pageId } = action.payload; yield put(setAppMode(APP_MODE.PUBLISHED)); + yield put(updateAppPersistentStore(getPersistentAppStore(applicationId))); yield put({ type: ReduxActionTypes.START_EVALUATION }); yield all([ // TODO (hetu) Remove spl view call for fetch actions @@ -185,7 +185,6 @@ export function* initializeAppViewerSaga( return; } - yield put(updateAppStore(getAppStore(applicationId))); const defaultPageId = yield select(getDefaultPageId); const toLoadPageId = pageId || defaultPageId; @@ -212,7 +211,6 @@ export function* initializeAppViewerSaga( } yield put(setAppMode(APP_MODE.PUBLISHED)); - yield put(updateAppStore(getAppStore(applicationId))); yield put({ type: ReduxActionTypes.INITIALIZE_PAGE_VIEWER_SUCCESS, diff --git a/app/client/src/sagas/userSagas.tsx b/app/client/src/sagas/userSagas.tsx index 4e707690c4..dff433c629 100644 --- a/app/client/src/sagas/userSagas.tsx +++ b/app/client/src/sagas/userSagas.tsx @@ -338,7 +338,7 @@ export function* logoutSaga() { if (isValidResponse) { AnalyticsUtil.reset(); yield put(logoutUserSuccess()); - localStorage.removeItem("THEME"); + localStorage.clear(); yield put(flushErrorsAndRedirect(AUTH_LOGIN_URL)); } } catch (error) { diff --git a/app/client/src/selectors/entitiesSelector.ts b/app/client/src/selectors/entitiesSelector.ts index 23a549eb14..6ac43748ae 100644 --- a/app/client/src/selectors/entitiesSelector.ts +++ b/app/client/src/selectors/entitiesSelector.ts @@ -12,6 +12,7 @@ import { find } from "lodash"; import ImageAlt from "assets/images/placeholder-image.svg"; import { CanvasWidgetsReduxState } from "../reducers/entityReducers/canvasWidgetsReducer"; import { MAIN_CONTAINER_WIDGET_ID } from "constants/WidgetConstants"; +import { AppStoreState } from "reducers/entityReducers/appReducer"; export const getEntities = (state: AppState): AppState["entities"] => state.entities; @@ -282,6 +283,9 @@ export const isActionDirty = (id: string) => export const getAppData = (state: AppState) => state.entities.app; +export const getAppStoreData = (state: AppState): AppStoreState => + state.entities.app.store; + export const getCanvasWidgets = (state: AppState): CanvasWidgetsReduxState => state.entities.canvasWidgets; diff --git a/app/client/src/utils/localStorage.test.ts b/app/client/src/utils/localStorage.test.ts new file mode 100644 index 0000000000..5542e5b6d6 --- /dev/null +++ b/app/client/src/utils/localStorage.test.ts @@ -0,0 +1,29 @@ +import myLocalStorage from "utils/localStorage"; + +describe("local storage", () => { + it("calls getItem", () => { + jest.spyOn(window.localStorage.__proto__, "getItem"); + window.localStorage.__proto__.getItem = jest.fn(); + myLocalStorage.getItem("myTestKey"); + expect(localStorage.getItem).toBeCalledWith("myTestKey"); + }); + it("calls setItem", () => { + jest.spyOn(window.localStorage.__proto__, "setItem"); + window.localStorage.__proto__.setItem = jest.fn(); + myLocalStorage.setItem("myTestKey", "testValue"); + expect(localStorage.setItem).toBeCalledWith("myTestKey", "testValue"); + }); + + it("calls removeItem", () => { + jest.spyOn(window.localStorage.__proto__, "removeItem"); + window.localStorage.__proto__.removeItem = jest.fn(); + myLocalStorage.removeItem("myTestKey"); + expect(localStorage.removeItem).toBeCalledWith("myTestKey"); + }); + it("calls clear", () => { + jest.spyOn(window.localStorage.__proto__, "clear"); + window.localStorage.__proto__.clear = jest.fn(); + myLocalStorage.clear(); + expect(localStorage.clear).toBeCalled(); + }); +}); diff --git a/app/client/src/utils/localStorage.tsx b/app/client/src/utils/localStorage.tsx index 59b32de756..f78e84bda3 100644 --- a/app/client/src/utils/localStorage.tsx +++ b/app/client/src/utils/localStorage.tsx @@ -52,6 +52,14 @@ const getLocalStorage = () => { } }; + const clear = () => { + try { + storage.clear(); + } catch (e) { + handleError(e); + } + }; + const isSupported = () => !!window.localStorage; return { @@ -59,6 +67,7 @@ const getLocalStorage = () => { setItem, removeItem, isSupported, + clear, }; }; diff --git a/app/client/src/workers/evaluationUtils.test.ts b/app/client/src/workers/evaluationUtils.test.ts index 6e8b9eaacf..a5a9e2ef73 100644 --- a/app/client/src/workers/evaluationUtils.test.ts +++ b/app/client/src/workers/evaluationUtils.test.ts @@ -1,4 +1,6 @@ -import { getAllPaths } from "./evaluationUtils"; +import { addFunctions, getAllPaths } from "./evaluationUtils"; +import { DataTree, ENTITY_TYPE } from "entities/DataTree/dataTreeFactory"; +import { PluginType } from "entities/Action"; describe("getAllPaths", () => { it("getsAllPaths", () => { @@ -39,3 +41,177 @@ describe("getAllPaths", () => { expect(actual).toStrictEqual(result); }); }); + +describe("Add functions", () => { + it("adds functions correctly", () => { + const dataTree: DataTree = { + action1: { + actionId: "123", + data: {}, + config: {}, + pluginType: PluginType.API, + dynamicBindingPathList: [], + name: "action1", + bindingPaths: {}, + isLoading: false, + run: {}, + ENTITY_TYPE: ENTITY_TYPE.ACTION, + }, + }; + const dataTreeWithFunctions = addFunctions(dataTree); + expect(dataTreeWithFunctions.actionPaths).toStrictEqual([ + "action1.run", + "navigateTo", + "showAlert", + "showModal", + "closeModal", + "storeValue", + "download", + "copyToClipboard", + "resetWidget", + ]); + + // Action run + const onSuccess = "() => {successRun()}"; + const onError = "() => {failureRun()}"; + const actionParams = "{ param1: value1 }"; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const actionRunResponse = dataTreeWithFunctions.action1.run( + onSuccess, + onError, + actionParams, + ); + expect(actionRunResponse).toStrictEqual({ + type: "RUN_ACTION", + payload: { + actionId: "123", + onSuccess: `{{${onSuccess}}}`, + onError: `{{${onError}}}`, + params: actionParams, + }, + }); + + // Navigate To + const pageNameOrUrl = "www.google.com"; + const params = "{ param1: value1 }"; + const target = "NEW_WINDOW"; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const navigateToResponse = dataTreeWithFunctions.navigateTo( + pageNameOrUrl, + params, + target, + ); + expect(navigateToResponse).toStrictEqual({ + type: "NAVIGATE_TO", + payload: { + pageNameOrUrl, + params, + target, + }, + }); + + // Show alert + const message = "Alert message"; + const style = "info"; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const showAlertResponse = dataTreeWithFunctions.showAlert(message, style); + expect(showAlertResponse).toStrictEqual({ + type: "SHOW_ALERT", + payload: { + message, + style, + }, + }); + + // Show Modal + const modalName = "Modal 1"; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const showModalResponse = dataTreeWithFunctions.showModal(modalName); + expect(showModalResponse).toStrictEqual({ + type: "SHOW_MODAL_BY_NAME", + payload: { + modalName, + }, + }); + + // Close Modal + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const closeModalResponse = dataTreeWithFunctions.closeModal(modalName); + expect(closeModalResponse).toStrictEqual({ + type: "CLOSE_MODAL", + payload: { + modalName, + }, + }); + + // Store value + const key = "some"; + const value = "thing"; + const persist = false; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const storeValueResponse = dataTreeWithFunctions.storeValue( + key, + value, + persist, + ); + expect(storeValueResponse).toStrictEqual({ + type: "STORE_VALUE", + payload: { + key, + value, + persist, + }, + }); + + // Download + const data = "file"; + const name = "downloadedFile.txt"; + const type = "text"; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const downloadResponse = dataTreeWithFunctions.download(data, name, type); + expect(downloadResponse).toStrictEqual({ + type: "DOWNLOAD", + payload: { + data, + name, + type, + }, + }); + + // copy to clipboard + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const copyToClipboardResponse = dataTreeWithFunctions.copyToClipboard(data); + expect(copyToClipboardResponse).toStrictEqual({ + type: "COPY_TO_CLIPBOARD", + payload: { + data, + options: { debug: undefined, format: undefined }, + }, + }); + + // reset widget + const widgetName = "widget1"; + const resetChildren = true; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const resetWidgetResponse = dataTreeWithFunctions.resetWidget( + widgetName, + resetChildren, + ); + expect(resetWidgetResponse).toStrictEqual({ + type: "RESET_WIDGET_META_RECURSIVE_BY_NAME", + payload: { + widgetName, + resetChildren, + }, + }); + }); +}); diff --git a/app/client/src/workers/evaluationUtils.ts b/app/client/src/workers/evaluationUtils.ts index bafed5579d..c92b6edce1 100644 --- a/app/client/src/workers/evaluationUtils.ts +++ b/app/client/src/workers/evaluationUtils.ts @@ -424,10 +424,14 @@ export const addFunctions = (dataTree: Readonly): DataTree => { }; withFunction.actionPaths.push("closeModal"); - withFunction.storeValue = function(key: string, value: string) { + withFunction.storeValue = function( + key: string, + value: string, + persist = true, + ) { return { type: "STORE_VALUE", - payload: { key, value }, + payload: { key, value, persist }, }; }; withFunction.actionPaths.push("storeValue");