fix: remove redundant eval trigger (#36433)

## Description

We removed the fetchPublishedPage action from the page switch flow in
App Viewer as the consolidated API now loads the migratedDSL.
- PageSaga code was refactored to make sure the `postFetchPublishedPage`
logic is common for both the init and switch page to ease maintenance.

Fixes #36237 

## Automation

/ok-to-test tags="@tag.All"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/11132731783>
> Commit: 026ef4e704ce878be75446d470eef1af842aff39
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11132731783&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Tue, 01 Oct 2024 21:32:17 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

## Summary by CodeRabbit

- **New Features**
- Enhanced handling of page fetching with the addition of new action
types for managing published page resources.
- Introduced new functions to streamline canvas layout updates and
post-fetch processes.

- **Bug Fixes**
- Resolved issues related to fetching published page resources and
improved error handling.

- **Refactor**
- Simplified the logic in the `PageSagas` for better organization and
reduced code duplication.
- Improved the control flow in the `AppViewer` component for fetching
page resources.

- **Tests**
- Updated tests to reflect changes in action payload structures and new
action types.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Rishabh Rathod 2024-10-02 13:27:44 +05:30 committed by GitHub
parent 4f501419cd
commit f49cf0a219
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 81 additions and 69 deletions

View File

@ -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,
},
});

View File

@ -981,6 +981,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 = {

View File

@ -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);
yield call(postFetchedPublishedPage, response, pageId);
// 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,
),
);
// 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({

View File

@ -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,
),
)

View File

@ -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 = [

View File

@ -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,
}),
);
}
}
}

View File

@ -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>,