Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: updated actions fetch logic for consolidated view api #36096

Merged
merged 8 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
agHelper,
assertHelper,
deployMode,
homePage,
locators,
Expand All @@ -17,6 +18,8 @@ describe(
agHelper.AssertElementVisibility(locators._widgetInCanvas("textwidget"));
deployMode.DeployApp();

assertHelper.AssertNetworkStatus("@postExecute", 200, true);
sneha122 marked this conversation as resolved.
Show resolved Hide resolved

agHelper.GetNAssertContains(
locators._widgetInDeployed("textwidget"),
"User count :5",
Expand All @@ -37,6 +40,9 @@ describe(

agHelper.GetNClickByContains(locators._deployedPage, "Page2");

assertHelper.AssertNetworkStatus("@getConsolidatedData", 200, true);
assertHelper.AssertNetworkStatus("@postExecute", 200, true);

agHelper.GetNAssertContains(
locators._widgetInDeployed("textwidget"),
"User count :10",
Expand All @@ -55,6 +61,9 @@ describe(

agHelper.GetNClickByContains(locators._deployedPage, "Page1");

assertHelper.AssertNetworkStatus("@getConsolidatedData", 200, true);
assertHelper.AssertNetworkStatus("@postExecute", 200, true);

agHelper.GetNAssertContains(
locators._widgetInDeployed("textwidget"),
"User count :5",
Expand Down
15 changes: 15 additions & 0 deletions app/client/src/actions/pageActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ export interface FetchPublishedPageActionPayload {
pageWithMigratedDsl?: FetchPageResponse;
}

export interface FetchPublishedPageResourcesPayload {
pageId: string;
}

export const fetchPublishedPageAction = (
pageId: string,
bustCache = false,
Expand Down Expand Up @@ -290,6 +294,17 @@ 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> => ({
type: ReduxActionTypes.FETCH_PUBLISHED_PAGE_RESOURCES_INIT,
payload: {
pageId,
},
});

// update a page

export interface UpdatePageActionPayload {
Expand Down
2 changes: 2 additions & 0 deletions app/client/src/ce/constants/ReduxActionConstants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -980,6 +980,7 @@ const AppViewActionTypes = {
FETCH_JS_ACTIONS_VIEW_MODE_SUCCESS: "FETCH_JS_ACTIONS_VIEW_MODE_SUCCESS",
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",
};

const AppViewActionErrorTypes = {
Expand All @@ -988,6 +989,7 @@ const AppViewActionErrorTypes = {
PUBLISH_APPLICATION_ERROR: "PUBLISH_APPLICATION_ERROR",
FETCH_ACTIONS_VIEW_MODE_ERROR: "FETCH_ACTION_VIEW_MODE_ERROR",
FETCH_JS_ACTIONS_VIEW_MODE_ERROR: "FETCH_JS_ACTIONS_VIEW_MODE_ERROR",
FETCH_PUBLISHED_PAGE_RESOURCES_ERROR: "FETCH_PUBLISHED_PAGE_RESOURCES_ERROR",
};

const WorkspaceActionTypes = {
Expand Down
41 changes: 41 additions & 0 deletions app/client/src/ce/sagas/PageSagas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {
DeletePageActionPayload,
FetchPageActionPayload,
FetchPublishedPageActionPayload,
FetchPublishedPageResourcesPayload,
GenerateTemplatePageActionPayload,
SetPageOrderActionPayload,
SetupPageActionPayload,
Expand Down Expand Up @@ -87,6 +88,7 @@ import {
fetchActionsForPage,
fetchActionsForPageError,
fetchActionsForPageSuccess,
fetchActionsForView,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor suggestion for action constants

While the addition of fetchActionsForView is necessary for the new functionality, consider grouping related action constants together or using an enum for better organization and readability.

setActionsToExecuteOnPageLoad,
setJSActionsToExecuteOnPageLoad,
} from "actions/pluginActionActions";
Expand All @@ -113,6 +115,7 @@ import {
import WidgetFactory from "WidgetProvider/factory";
import { builderURL } from "ee/RouteBuilder";
import { failFastApiCalls, waitForWidgetConfigBuild } from "sagas/InitSagas";
import { type InitConsolidatedApi } from "sagas/InitSagas";
import { resizePublishedMainCanvasToLowestWidget } from "sagas/WidgetOperationUtils";
import {
checkAndLogErrorsIfCyclicDependency,
Expand Down Expand Up @@ -146,6 +149,7 @@ import type { LayoutSystemTypes } from "layoutSystems/types";
import { getIsAnvilLayout } from "layoutSystems/anvil/integrations/selectors";
import { convertToBasePageIdSelector } from "selectors/pageListSelectors";
import type { Page } from "entities/Page";
import ConsolidatedPageLoadApi from "api/ConsolidatedPageLoadApi";

export const checkIfMigrationIsNeeded = (
fetchPageResponse?: FetchPageResponse,
Expand Down Expand Up @@ -378,6 +382,43 @@ export function* fetchPublishedPageSaga(
}
}

export function* fetchPublishedPageResourcesSaga(
sneha122 marked this conversation as resolved.
Show resolved Hide resolved
action: ReduxAction<FetchPublishedPageResourcesPayload>,
) {
try {
const { pageId } = action.payload;

const params = { defaultPageId: pageId };
const initConsolidatedApiResponse: ApiResponse<InitConsolidatedApi> =
yield ConsolidatedPageLoadApi.getConsolidatedPageLoadDataView(params);

const isValidResponse: boolean = yield validateResponse(
initConsolidatedApiResponse,
);
const response: InitConsolidatedApi | undefined =
initConsolidatedApiResponse.data;

if (isValidResponse) {
// We need to recall consolidated view API in order to fetch actions when page is switched
// As in the first call only actions of the current page are fetched
// 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 put(fetchActionsForView({ applicationId: "", publishedActions }));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sneha122 if we send the applicationId here, in that case also because publishedActions is present it won't be fetching the actions. correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, as long as we have data in publishedActions, it won't fetch actions again

yield put(fetchAllPageEntityCompletion([executePageLoadActions()]));
}
} catch (error) {
yield put({
type: ReduxActionErrorTypes.FETCH_PUBLISHED_PAGE_RESOURCES_ERROR,
payload: {
error,
},
});
}
}

export function* fetchAllPublishedPagesSaga() {
try {
const pageIdentities: { pageId: string; basePageId: string }[] =
Expand Down
5 changes: 5 additions & 0 deletions app/client/src/ee/sagas/PageSagas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
refreshTheApp,
setupPageSaga,
setupPublishedPageSaga,
fetchPublishedPageResourcesSaga,
} from "ce/sagas/PageSagas";
import {
all,
Expand Down Expand Up @@ -72,5 +73,9 @@ export default function* pageSagas() {
ReduxActionTypes.SETUP_PUBLISHED_PAGE_INIT,
setupPublishedPageSaga,
),
takeLatest(
ReduxActionTypes.FETCH_PUBLISHED_PAGE_RESOURCES_INIT,
fetchPublishedPageResourcesSaga,
),
]);
}
8 changes: 7 additions & 1 deletion app/client/src/pages/AppViewer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ import { useSelector } from "react-redux";
import BrandingBadge from "./BrandingBadge";
import { setAppViewHeaderHeight } from "actions/appViewActions";
import { CANVAS_SELECTOR } from "constants/WidgetConstants";
import { setupPublishedPage } from "actions/pageActions";
import {
setupPublishedPage,
fetchPublishedPageResourcesAction,
} from "actions/pageActions";
import usePrevious from "utils/hooks/usePrevious";
import { getIsBranchUpdated } from "../utils";
import { APP_MODE } from "entities/App";
Expand Down Expand Up @@ -168,6 +171,9 @@ function AppViewer(props: Props) {
)?.pageId;
if (pageId) {
dispatch(setupPublishedPage(pageId, true));

// Used for fetching page resources
dispatch(fetchPublishedPageResourcesAction(basePageId));
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions app/client/src/reducers/uiReducers/appViewReducer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ const appViewReducer = createReducer(initialState, {
) => {
return { ...state, isFetchingPage: false };
},
[ReduxActionErrorTypes.FETCH_PUBLISHED_PAGE_RESOURCES_ERROR]: (
state: AppViewReduxState,
) => {
return { ...state, isFetchingPage: false };
},
[ReduxActionTypes.FETCH_PUBLISHED_PAGE_SUCCESS]: (
state: AppViewReduxState,
) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,10 @@ Flux<NewAction> findAllByApplicationIdAndViewMode(
Flux<NewAction> findAllByApplicationIdAndViewMode(
String applicationId, Boolean viewMode, Optional<AclPermission> permission, Optional<Sort> sort);

Flux<NewAction> findAllByApplicationIdAndPluginType(
String applicationId, Boolean viewMode, AclPermission permission, Sort sort, List<String> pluginTypes);

Flux<ActionViewDTO> getActionsForViewMode(String applicationId);

Flux<ActionViewDTO> getActionsForViewModeByPageId(String pageId);

ActionViewDTO generateActionViewDTO(NewAction action, ActionDTO actionDTO, boolean viewMode);

Mono<ActionDTO> deleteUnpublishedAction(String id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -729,19 +729,6 @@ public Flux<NewAction> findByPageIdAndViewMode(String pageId, Boolean viewMode,
return repository.findByPageIdAndViewMode(pageId, viewMode, permission).flatMap(this::sanitizeAction);
}

@Override
public Flux<NewAction> findAllByApplicationIdAndPluginType(
String applicationId,
Boolean viewMode,
AclPermission permission,
Sort sort,
List<String> excludedPluginTypes) {
return repository
.findPublishedActionsByApplicationIdAndPluginType(applicationId, excludedPluginTypes, permission, sort)
.name(VIEW_MODE_FETCH_ACTIONS_FROM_DB)
.tap(Micrometer.observation(observationRegistry));
}

@Override
public Flux<NewAction> findAllByApplicationIdAndViewMode(
String applicationId, Boolean viewMode, AclPermission permission, Sort sort) {
Expand Down Expand Up @@ -776,19 +763,40 @@ public Flux<NewAction> findAllByApplicationIdAndViewMode(
.flatMapMany(this::addMissingPluginDetailsIntoAllActions);
}

@Override
public Flux<ActionViewDTO> getActionsForViewMode(String applicationId) {

if (applicationId == null || applicationId.isEmpty()) {
return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.APPLICATION_ID));
return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.PAGE_ID));
}

List<String> excludedPluginTypes = List.of(PluginType.JS.toString());

// fetch the published actions by appId
// No need to sort the results
return repository
.findPublishedActionsByAppIdAndExcludedPluginType(
applicationId, excludedPluginTypes, actionPermission.getExecutePermission(), null)
.name(VIEW_MODE_FETCH_ACTIONS_FROM_DB)
.tap(Micrometer.observation(observationRegistry))
.map(action -> generateActionViewDTO(action, action.getPublishedAction(), true));
}
sneha122 marked this conversation as resolved.
Show resolved Hide resolved

@Override
public Flux<ActionViewDTO> getActionsForViewModeByPageId(String pageId) {

if (pageId == null || pageId.isEmpty()) {
return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.PAGE_ID));
}

List<String> excludedPluginTypes = List.of(PluginType.JS.toString());

// fetch the published actions by applicationId
// fetch the published actions by pageId
// No need to sort the results
return findAllByApplicationIdAndPluginType(
applicationId, true, actionPermission.getExecutePermission(), null, excludedPluginTypes)
return repository
.findPublishedActionsByPageIdAndExcludedPluginType(
pageId, excludedPluginTypes, actionPermission.getExecutePermission(), null)
.name(VIEW_MODE_FETCH_ACTIONS_FROM_DB)
.tap(Micrometer.observation(observationRegistry))
.map(action -> generateActionViewDTO(action, action.getPublishedAction(), true));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ Flux<NewAction> findAllActionsByNameAndPageIdsAndViewMode(

Flux<NewAction> findByApplicationId(String applicationId, AclPermission aclPermission, Sort sort);

Flux<NewAction> findPublishedActionsByApplicationIdAndPluginType(
String applicationId, List<String> pluginTypes, AclPermission aclPermission, Sort sort);
Flux<NewAction> findPublishedActionsByPageIdAndExcludedPluginType(
String pageId, List<String> pluginTypes, AclPermission aclPermission, Sort sort);

Flux<NewAction> findPublishedActionsByAppIdAndExcludedPluginType(
String appId, List<String> pluginTypes, AclPermission aclPermission, Sort sort);

Flux<NewAction> findByApplicationId(
String applicationId, Optional<AclPermission> aclPermission, Optional<Sort> sort);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,25 +202,43 @@ protected BridgeQuery<NewAction> getCriterionForFindByApplicationId(String appli
return Bridge.equal(NewAction.Fields.applicationId, applicationId);
}

@Override
public Flux<NewAction> findPublishedActionsByApplicationIdAndPluginType(
public Flux<NewAction> findPublishedActionsByAppIdAndExcludedPluginType(
String applicationId, List<String> excludedPluginTypes, AclPermission aclPermission, Sort sort) {
return queryBuilder()
.criteria(getCriterionForFindPublishedActionsByApplicationIdAndPluginType(
.criteria(getCriterionForFindPublishedActionsByAppIdAndExcludedPluginType(
sneha122 marked this conversation as resolved.
Show resolved Hide resolved
applicationId, excludedPluginTypes))
.permission(aclPermission)
.sort(sort)
.all();
}

protected BridgeQuery<NewAction> getCriterionForFindPublishedActionsByApplicationIdAndPluginType(
protected BridgeQuery<NewAction> getCriterionForFindPublishedActionsByAppIdAndExcludedPluginType(
String applicationId, List<String> excludedPluginTypes) {
final BridgeQuery<NewAction> q = getCriterionForFindByApplicationId(applicationId);
q.and(Bridge.or(
Bridge.notIn(NewAction.Fields.pluginType, excludedPluginTypes),
Bridge.isNull(NewAction.Fields.pluginType)));

q.isNotNull(NewAction.Fields.publishedAction_pageId);
return q;
}

@Override
public Flux<NewAction> findPublishedActionsByPageIdAndExcludedPluginType(
String pageId, List<String> excludedPluginTypes, AclPermission aclPermission, Sort sort) {
return queryBuilder()
.criteria(getCriterionForFindPublishedActionsByPageIdAndExcludedPluginType(pageId, excludedPluginTypes))
.permission(aclPermission)
.sort(sort)
.all();
}

protected BridgeQuery<NewAction> getCriterionForFindPublishedActionsByPageIdAndExcludedPluginType(
String pageId, List<String> excludedPluginTypes) {
final BridgeQuery<NewAction> q = Bridge.equal(NewAction.Fields.publishedAction_pageId, pageId);
q.and(Bridge.or(
Bridge.notIn(NewAction.Fields.pluginType, excludedPluginTypes),
Bridge.isNull(NewAction.Fields.pluginType)));

return q;
}
Expand Down
Loading
Loading