fix: remove redundant eval trigger (#36764)

Fixes #36237 ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results  > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/11267830066> > Commit: 026ef4e > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11267830066&attempt=3" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Thu, 10 Oct 2024 09:40:31 UTC  ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No  ## Summary by CodeRabbit ## Release Notes - **New Features** - Introduced new action types for fetching published page resources, enhancing state management during the fetch process. - Streamlined the resource-fetching logic in the AppViewer component for improved efficiency. - **Bug Fixes** - Updated action handlers to accurately reflect the fetching state for published page resources. - **Refactor** - Simplified parameters for several functions related to page setup and fetching, promoting better clarity and maintainability. - **Tests** - Adjusted test cases to align with the updated action payload structure for fetching published pages.
appsmithorg · Oct 10, 2024 · 00981e3 · 00981e3
1 parent 9041c48
commit 00981e3
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 69 deletions.
diff --git a/app/client/src/actions/pageActions.tsx b/app/client/src/actions/pageActions.tsx
@@ -68,25 +68,23 @@ export const fetchPageAction = (
 export interface FetchPublishedPageActionPayload {
   pageId: string;
   bustCache?: boolean;
-  firstLoad?: boolean;
   pageWithMigratedDsl?: FetchPageResponse;
 }
 
 export interface FetchPublishedPageResourcesPayload {
   pageId: string;
+  basePageId: string;
 }
 
 export const fetchPublishedPageAction = (
   pageId: string,
   bustCache = false,
-  firstLoad = false,
   pageWithMigratedDsl?: FetchPageResponse,
 ): ReduxAction<FetchPublishedPageActionPayload> => ({
   type: ReduxActionTypes.FETCH_PUBLISHED_PAGE_INIT,
   payload: {
     pageId,
     bustCache,
-    firstLoad,
     pageWithMigratedDsl,
   },
 });
@@ -299,12 +297,14 @@ export const clonePageSuccess = ({
 
 // Fetches resources required for published page, currently only used for fetching actions
 // In future we can reuse this for fetching other page level resources in published mode
-export const fetchPublishedPageResourcesAction = (
-  pageId: string,
-): ReduxAction<FetchPublishedPageResourcesPayload> => ({
+export const fetchPublishedPageResources = ({
+  basePageId,
+  pageId,
+}: FetchPublishedPageResourcesPayload): ReduxAction<FetchPublishedPageResourcesPayload> => ({
   type: ReduxActionTypes.FETCH_PUBLISHED_PAGE_RESOURCES_INIT,
   payload: {
     pageId,
+    basePageId,
   },
 });
 
@@ -675,21 +675,18 @@ export const setupPageAction = (
 export interface SetupPublishedPageActionPayload {
   pageId: string;
   bustCache: boolean;
-  firstLoad: boolean;
   pageWithMigratedDsl?: FetchPageResponse;
 }
 
 export const setupPublishedPage = (
   pageId: string,
   bustCache = false,
-  firstLoad = false,
   pageWithMigratedDsl?: FetchPageResponse,
 ): ReduxAction<SetupPublishedPageActionPayload> => ({
   type: ReduxActionTypes.SETUP_PUBLISHED_PAGE_INIT,
   payload: {
     pageId,
     bustCache,
-    firstLoad,
     pageWithMigratedDsl,
   },
 });
diff --git a/app/client/src/ce/constants/ReduxActionConstants.tsx b/app/client/src/ce/constants/ReduxActionConstants.tsx
@@ -985,6 +985,8 @@ const AppViewActionTypes = {
   SET_APP_VIEWER_HEADER_HEIGHT: "SET_APP_VIEWER_HEADER_HEIGHT",
   SET_APP_SIDEBAR_PINNED: "SET_APP_SIDEBAR_PINNED",
   FETCH_PUBLISHED_PAGE_RESOURCES_INIT: "FETCH_PUBLISHED_PAGE_RESOURCES_INIT",
+  FETCH_PUBLISHED_PAGE_RESOURCES_SUCCESS:
+    "FETCH_PUBLISHED_PAGE_RESOURCES_SUCCESS",
 };
 
 const AppViewActionErrorTypes = {

diff --git a/app/client/src/ce/sagas/PageSagas.tsx b/app/client/src/ce/sagas/PageSagas.tsx
@@ -325,12 +325,47 @@ export function* fetchPageSaga(action: ReduxAction<FetchPageActionPayload>) {
   }
 }
 
+export function* updateCanvasLayout(response: FetchPageResponse) {
+  // Wait for widget config to load before we can get the canvas payload
+  yield call(waitForWidgetConfigBuild);
+  // Get Canvas payload
+  const canvasWidgetsPayload = getCanvasWidgetsPayload(response);
+
+  // resize main canvas
+  resizePublishedMainCanvasToLowestWidget(canvasWidgetsPayload.widgets);
+  // Update the canvas
+  yield put(initCanvasLayout(canvasWidgetsPayload));
+
+  // Since new page has new layout, we need to generate a data structure
+  // to compute dynamic height based on the new layout.
+  yield put(generateAutoHeightLayoutTreeAction(true, true));
+}
+
+export function* postFetchedPublishedPage(
+  response: FetchPageResponse,
+  pageId: string,
+) {
+  // set current page
+  yield put(
+    updateCurrentPage(
+      pageId,
+      response.data.slug,
+      response.data.userPermissions,
+    ),
+  );
+  // Clear any existing caches
+  yield call(clearEvalCache);
+  // Set url params
+  yield call(setDataUrl);
+
+  yield call(updateCanvasLayout, response);
+}
+
 export function* fetchPublishedPageSaga(
   action: ReduxAction<FetchPublishedPageActionPayload>,
 ) {
   try {
-    const { bustCache, firstLoad, pageId, pageWithMigratedDsl } =
-      action.payload;
+    const { bustCache, pageId, pageWithMigratedDsl } = action.payload;
 
     const params = { pageId, bustCache };
     const response: FetchPageResponse = yield call(
@@ -342,41 +377,9 @@ export function* fetchPublishedPageSaga(
     const isValidResponse: boolean = yield validateResponse(response);
 
     if (isValidResponse) {
-      // Clear any existing caches
-      yield call(clearEvalCache);
-      // Set url params
-      yield call(setDataUrl);
-      // Wait for widget config to load before we can get the canvas payload
-      yield call(waitForWidgetConfigBuild);
-      // Get Canvas payload
-      const canvasWidgetsPayload = getCanvasWidgetsPayload(response);
-
-      // resize main canvas
-      resizePublishedMainCanvasToLowestWidget(canvasWidgetsPayload.widgets);
-      // Update the canvas
-      yield put(initCanvasLayout(canvasWidgetsPayload));
-      // set current page
-      yield put(
-        updateCurrentPage(
-          pageId,
-          response.data.slug,
-          response.data.userPermissions,
-        ),
-      );
+      yield call(postFetchedPublishedPage, response, pageId);
 
-      // dispatch fetch page success
       yield put(fetchPublishedPageSuccess());
-
-      // Since new page has new layout, we need to generate a data structure
-      // to compute dynamic height based on the new layout.
-      yield put(generateAutoHeightLayoutTreeAction(true, true));
-
-      /* Currently, All Actions are fetched in initSagas and on pageSwitch we only fetch page
-       */
-      // Hence, if is not isFirstLoad then trigger evaluation with execute pageLoad action
-      if (!firstLoad) {
-        yield put(fetchAllPageEntityCompletion([executePageLoadActions()]));
-      }
     }
   } catch (error) {
     yield put({
@@ -392,9 +395,9 @@ export function* fetchPublishedPageResourcesSaga(
   action: ReduxAction<FetchPublishedPageResourcesPayload>,
 ) {
   try {
-    const { pageId } = action.payload;
+    const { basePageId, pageId } = action.payload;
 
-    const params = { defaultPageId: pageId };
+    const params = { defaultPageId: basePageId };
     const initConsolidatedApiResponse: ApiResponse<InitConsolidatedApi> =
       yield ConsolidatedPageLoadApi.getConsolidatedPageLoadDataView(params);
 
@@ -410,10 +413,18 @@ export function* fetchPublishedPageResourcesSaga(
       // In future, we can reuse this saga to fetch other resources of the page like actionCollections etc
       const { publishedActions } = response;
 
-      // Sending applicationId as empty as we have publishedActions present,
-      // it won't call the actions view api with applicationId
+      yield call(
+        postFetchedPublishedPage,
+        response.pageWithMigratedDsl,
+        pageId,
+      );
+
+      // NOTE: fetchActionsForView is used here to update publishedActions in redux store and not to fetch actions again
       yield put(fetchActionsForView({ applicationId: "", publishedActions }));
       yield put(fetchAllPageEntityCompletion([executePageLoadActions()]));
+      yield put({
+        type: ReduxActionTypes.FETCH_PUBLISHED_PAGE_RESOURCES_SUCCESS,
+      });
     }
   } catch (error) {
     yield put({
@@ -1425,21 +1436,13 @@ export function* setupPublishedPageSaga(
   action: ReduxAction<SetupPublishedPageActionPayload>,
 ) {
   try {
-    const { bustCache, firstLoad, pageId, pageWithMigratedDsl } =
-      action.payload;
+    const { bustCache, pageId, pageWithMigratedDsl } = action.payload;
 
     /*
       Added the first line for isPageSwitching redux state to be true when page is being fetched to fix scroll position issue.
       Added the second line for sync call instead of async (due to first line) as it was leading to issue with on page load actions trigger.
     */
-    yield put(
-      fetchPublishedPageAction(
-        pageId,
-        bustCache,
-        firstLoad,
-        pageWithMigratedDsl,
-      ),
-    );
+    yield put(fetchPublishedPageAction(pageId, bustCache, pageWithMigratedDsl));
     yield take(ReduxActionTypes.FETCH_PUBLISHED_PAGE_SUCCESS);
 
     yield put({

diff --git a/app/client/src/ce/sagas/__tests__/PageSaga.test.ts b/app/client/src/ce/sagas/__tests__/PageSaga.test.ts
@@ -47,7 +47,6 @@ describe("ce/PageSaga", () => {
         pageWithMigratedDsl: mockResponse.data
           .pageWithMigratedDsl as FetchPageResponse,
         bustCache: false,
-        firstLoad: true,
       },
     };
 
@@ -57,7 +56,6 @@ describe("ce/PageSaga", () => {
         fetchPublishedPageAction(
           action.payload.pageId,
           action.payload.bustCache,
-          action.payload.firstLoad,
           action.payload.pageWithMigratedDsl,
         ),
       )

diff --git a/app/client/src/entities/Engine/AppViewerEngine.ts b/app/client/src/entities/Engine/AppViewerEngine.ts
@@ -105,7 +105,7 @@ export default class AppViewerEngine extends AppEngine {
       }),
       fetchSelectedAppThemeAction(applicationId, currentTheme),
       fetchAppThemesAction(applicationId, themes),
-      setupPublishedPage(toLoadPageId, true, true, pageWithMigratedDsl),
+      setupPublishedPage(toLoadPageId, true, pageWithMigratedDsl),
     ];
 
     const successActionEffects = [

diff --git a/app/client/src/pages/AppViewer/index.tsx b/app/client/src/pages/AppViewer/index.tsx
@@ -28,10 +28,7 @@ import { useSelector } from "react-redux";
 import BrandingBadge from "./BrandingBadge";
 import { setAppViewHeaderHeight } from "actions/appViewActions";
 import { CANVAS_SELECTOR } from "constants/WidgetConstants";
-import {
-  setupPublishedPage,
-  fetchPublishedPageResourcesAction,
-} from "actions/pageActions";
+import { fetchPublishedPageResources } from "actions/pageActions";
 import usePrevious from "utils/hooks/usePrevious";
 import { getIsBranchUpdated } from "../utils";
 import { APP_MODE } from "entities/App";
@@ -165,10 +162,12 @@ function AppViewer(props: Props) {
         )?.pageId;
 
         if (pageId) {
-          dispatch(setupPublishedPage(pageId, true));
-
-          // Used for fetching page resources
-          dispatch(fetchPublishedPageResourcesAction(basePageId));
+          dispatch(
+            fetchPublishedPageResources({
+              basePageId,
+              pageId,
+            }),
+          );
         }
       }
     }

diff --git a/app/client/src/reducers/uiReducers/appViewReducer.tsx b/app/client/src/reducers/uiReducers/appViewReducer.tsx
@@ -26,6 +26,11 @@ const appViewReducer = createReducer(initialState, {
   [ReduxActionTypes.FETCH_PUBLISHED_PAGE_INIT]: (state: AppViewReduxState) => {
     return { ...state, isFetchingPage: true };
   },
+  [ReduxActionTypes.FETCH_PUBLISHED_PAGE_RESOURCES_INIT]: (
+    state: AppViewReduxState,
+  ) => {
+    return { ...state, isFetchingPage: true };
+  },
   [ReduxActionErrorTypes.FETCH_PUBLISHED_PAGE_ERROR]: (
     state: AppViewReduxState,
   ) => {
@@ -44,6 +49,14 @@ const appViewReducer = createReducer(initialState, {
       isFetchingPage: false,
     };
   },
+  [ReduxActionTypes.FETCH_PUBLISHED_PAGE_RESOURCES_SUCCESS]: (
+    state: AppViewReduxState,
+  ) => {
+    return {
+      ...state,
+      isFetchingPage: false,
+    };
+  },
   [ReduxActionTypes.SET_APP_VIEWER_HEADER_HEIGHT]: (
     state: AppViewReduxState,
     action: ReduxAction<number>,