From 9649d8561c406e9fe8e493e389b784f0ebc1a99a Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Mon, 22 Apr 2024 08:08:45 +0530 Subject: [PATCH] chore: Remove unneeded fields in snapshot response body (#32813) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These extra fields don't hold much semantic value, and are completely ignored by the client. This PR removes them and only keeps the one field that's used by the client. /ok-to-test tags="@tag.All" > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 9b680799db0d30fb56c908f02260abbb4b590ac3 > Cypress dashboard url: Click here! ## Summary by CodeRabbit - **Refactor** - Improved consistency in naming conventions related to snapshot details across the platform. - Enhanced data handling in snapshot fetching and updating processes for better performance. - Streamlined snapshot-related data structures and service responses to focus on relevant information. --- app/client/src/actions/autoLayoutActions.ts | 6 +++--- .../uiReducers/layoutConversionReducer.ts | 8 +++---- app/client/src/sagas/SnapshotSagas.ts | 18 +++++----------- .../src/selectors/autoLayoutSelectors.tsx | 2 +- .../ApplicationSnapshotResponseDTO.java | 21 +------------------ .../ce/ApplicationSnapshotServiceCEImpl.java | 2 +- .../ApplicationSnapshotRepositoryTest.java | 2 -- .../ApplicationSnapshotServiceTest.java | 3 --- 8 files changed, 15 insertions(+), 47 deletions(-) diff --git a/app/client/src/actions/autoLayoutActions.ts b/app/client/src/actions/autoLayoutActions.ts index a2b5e9f457..4bcd9b38b2 100644 --- a/app/client/src/actions/autoLayoutActions.ts +++ b/app/client/src/actions/autoLayoutActions.ts @@ -3,7 +3,7 @@ import type { LayoutSystemTypes } from "layoutSystems/types"; import type { CanvasWidgetsReduxState } from "reducers/entityReducers/canvasWidgetsReducer"; import type { CONVERSION_STATES, - SnapShotDetails, + SnapshotDetails, } from "reducers/uiReducers/layoutConversionReducer"; /** @@ -46,11 +46,11 @@ export const setLayoutConversionStateAction = ( }; export const updateSnapshotDetails = ( - snapShotDetails: SnapShotDetails | undefined, + snapshotDetails: SnapshotDetails | undefined, ) => { return { type: ReduxActionTypes.UPDATE_SNAPSHOT_DETAILS, - payload: snapShotDetails, + payload: snapshotDetails, }; }; export function updateWidgetDimensionAction( diff --git a/app/client/src/reducers/uiReducers/layoutConversionReducer.ts b/app/client/src/reducers/uiReducers/layoutConversionReducer.ts index 5f1e4e948c..72481ae35d 100644 --- a/app/client/src/reducers/uiReducers/layoutConversionReducer.ts +++ b/app/client/src/reducers/uiReducers/layoutConversionReducer.ts @@ -21,8 +21,8 @@ export enum CONVERSION_STATES { RESTORING_SNAPSHOT_SPINNER = "RESTORING_SNAPSHOT_SPINNER", } -export interface SnapShotDetails { - lastUpdatedTime: string; +export interface SnapshotDetails { + updatedTime: string; } const initialState: layoutConversionReduxState = { @@ -58,14 +58,14 @@ const layoutConversionReducer = createImmerReducer(initialState, { }, [ReduxActionTypes.UPDATE_SNAPSHOT_DETAILS]: ( state: layoutConversionReduxState, - action: ReduxAction, + action: ReduxAction, ) => { state.snapshotDetails = action.payload; }, }); export interface layoutConversionReduxState { - snapshotDetails: SnapShotDetails | undefined; + snapshotDetails: SnapshotDetails | undefined; conversionError: Error | undefined; conversionState: CONVERSION_STATES; isConverting: boolean; diff --git a/app/client/src/sagas/SnapshotSagas.ts b/app/client/src/sagas/SnapshotSagas.ts index e26893bdd8..2d330679a9 100644 --- a/app/client/src/sagas/SnapshotSagas.ts +++ b/app/client/src/sagas/SnapshotSagas.ts @@ -7,7 +7,7 @@ import ApplicationApi from "@appsmith/api/ApplicationApi"; import type { PageDefaultMeta } from "@appsmith/api/ApplicationApi"; import { ReduxActionTypes } from "@appsmith/constants/ReduxActionConstants"; import log from "loglevel"; -import type { SnapShotDetails } from "reducers/uiReducers/layoutConversionReducer"; +import type { SnapshotDetails } from "reducers/uiReducers/layoutConversionReducer"; import { CONVERSION_STATES } from "reducers/uiReducers/layoutConversionReducer"; import { all, call, put, select, takeLatest } from "redux-saga/effects"; import { getCurrentApplicationId } from "selectors/editorSelectors"; @@ -43,7 +43,7 @@ export function* createSnapshotSaga() { //Saga to fetch application snapshot export function* fetchSnapshotSaga() { - let response: ApiResponse | undefined; + let response: ApiResponse | undefined; try { const applicationId: string = yield select(getCurrentApplicationId); response = yield ApplicationApi.getSnapShotDetails({ @@ -57,9 +57,7 @@ export function* fetchSnapshotSaga() { ); if (isValidResponse) { - const snapShotDetails = response?.data; - - return snapShotDetails; + return response?.data; } } catch (error) { if (getLogToSentryFromResponse(response)) { @@ -169,15 +167,9 @@ export function* deleteApplicationSnapshotSaga() { //Saga to update snapshot details by fetching info from backend function* updateSnapshotDetailsSaga() { try { - const snapShotDetails: { updatedTime: Date } | undefined = + const snapshotDetails: SnapshotDetails | undefined = yield call(fetchSnapshotSaga); - yield put( - updateSnapshotDetails( - snapShotDetails && snapShotDetails.updatedTime - ? { lastUpdatedTime: snapShotDetails.updatedTime?.toString() } - : undefined, - ), - ); + yield put(updateSnapshotDetails(snapshotDetails)); } catch (error) { throw error; } diff --git a/app/client/src/selectors/autoLayoutSelectors.tsx b/app/client/src/selectors/autoLayoutSelectors.tsx index ea2770eec0..c20029d98b 100644 --- a/app/client/src/selectors/autoLayoutSelectors.tsx +++ b/app/client/src/selectors/autoLayoutSelectors.tsx @@ -25,7 +25,7 @@ export const getFlexLayers = (parentId: string) => { }; export const getSnapshotUpdatedTime = (state: AppState) => - state.ui.layoutConversion.snapshotDetails?.lastUpdatedTime; + state.ui.layoutConversion.snapshotDetails?.updatedTime; export const getLayerIndex = (widgetId: string, parentId: string) => { return createSelector( diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/projections/ApplicationSnapshotResponseDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/projections/ApplicationSnapshotResponseDTO.java index 2eed4dfe6f..86ca6bf165 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/projections/ApplicationSnapshotResponseDTO.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/projections/ApplicationSnapshotResponseDTO.java @@ -5,29 +5,10 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonInclude; import java.time.Instant; -import java.util.Collections; -import java.util.List; - -public record ApplicationSnapshotResponseDTO( - @JsonIgnore String id, int chunkOrder, @JsonIgnore Instant updatedAt, @JsonIgnore Instant createdAt) { - - public ApplicationSnapshotResponseDTO() { - this(null, 0, null, null); - } - - @JsonInclude - public boolean isNew() { - return id == null; - } +public record ApplicationSnapshotResponseDTO(@JsonIgnore Instant updatedAt) { @JsonInclude public String getUpdatedTime() { return updatedAt == null ? null : DateUtils.ISO_FORMATTER.format(updatedAt); } - - // Likely not used by the client. Verify and remove if not needed. - @JsonInclude - public List getUserPermissions() { - return Collections.emptyList(); - } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCEImpl.java index 69ea3986a7..9f87c5d446 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationSnapshotServiceCEImpl.java @@ -74,7 +74,7 @@ public class ApplicationSnapshotServiceCEImpl implements ApplicationSnapshotServ .findBranchedApplicationId(branchName, applicationId, applicationPermission.getEditPermission()) .flatMap(branchedApplicationId -> applicationSnapshotRepository.findByApplicationIdAndChunkOrder(branchedApplicationId, 1)) - .defaultIfEmpty(new ApplicationSnapshotResponseDTO()); + .defaultIfEmpty(new ApplicationSnapshotResponseDTO(null)); } @Override diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ApplicationSnapshotRepositoryTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ApplicationSnapshotRepositoryTest.java index 10a515d0ce..25fcc546d9 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ApplicationSnapshotRepositoryTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ApplicationSnapshotRepositoryTest.java @@ -42,7 +42,6 @@ public class ApplicationSnapshotRepositoryTest { StepVerifier.create(snapshotMono) .assertNext(applicationSnapshot -> { assertThat(applicationSnapshot.updatedAt()).isNotNull(); - assertThat(applicationSnapshot.createdAt()).isNotNull(); }) .verifyComplete(); } @@ -67,7 +66,6 @@ public class ApplicationSnapshotRepositoryTest { StepVerifier.create(snapshotMono) .assertNext(applicationSnapshot -> { - assertThat(applicationSnapshot.createdAt()).isNotNull(); assertThat(applicationSnapshot.updatedAt()).isNotNull(); }) .verifyComplete(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationSnapshotServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationSnapshotServiceTest.java index 647a267f57..08dd28a190 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationSnapshotServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationSnapshotServiceTest.java @@ -106,7 +106,6 @@ public class ApplicationSnapshotServiceTest { StepVerifier.create(snapshotMono) .assertNext(snapshot -> { assertThat(snapshot.updatedAt()).isNotNull(); - assertThat(snapshot.createdAt()).isNotNull(); }) .verifyComplete(); } @@ -133,7 +132,6 @@ public class ApplicationSnapshotServiceTest { StepVerifier.create(snapshotMono) .assertNext(snapshot -> { assertThat(snapshot.updatedAt()).isNotNull(); - assertThat(snapshot.createdAt()).isNotNull(); }) .verifyComplete(); } @@ -165,7 +163,6 @@ public class ApplicationSnapshotServiceTest { .assertNext(objects -> { ApplicationSnapshotResponseDTO applicationSnapshot = objects.getT1(); assertThat(applicationSnapshot.updatedAt()).isNotNull(); - assertThat(applicationSnapshot.createdAt()).isNotNull(); }) .verifyComplete(); }