From 0a41a781ee04e26a7bfacc6912d3862da5c83aaf Mon Sep 17 00:00:00 2001 From: Nilansh Bansal Date: Tue, 24 Oct 2023 10:19:39 +0530 Subject: [PATCH] fix: git discard if applicationDetail already present (#28307) --- .../server/helpers/ImportExportUtils.java | 35 ++++- .../ImportApplicationServiceTests.java | 113 +++++++++++++ ...alid-application-with-app-positioning.json | 148 ++++++++++++++++++ 3 files changed, 294 insertions(+), 2 deletions(-) create mode 100644 app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-with-app-positioning.json diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java index b1493ac79a..055ea82be5 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java @@ -4,6 +4,7 @@ import com.appsmith.external.models.ActionDTO; import com.appsmith.external.models.Datasource; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.Application; +import com.appsmith.server.domains.ApplicationDetail; import com.appsmith.server.dtos.ApplicationJson; import lombok.extern.slf4j.Slf4j; import org.springframework.dao.InvalidDataAccessApiUsageException; @@ -79,9 +80,36 @@ public class ImportExportUtils { return ""; } + /** + * This function sets the current applicationDetail properties to null if the user wants to discard the changes + * and accept from the git repo which doesn't contain these. + * @param importedApplicationDetail + * @param existingApplicationDetail + */ + private static void setPropertiesToApplicationDetail( + ApplicationDetail importedApplicationDetail, ApplicationDetail existingApplicationDetail) { + // If the initial commit to git doesn't contain these keys and if we want to discard the changes, + // the function copyNestedNonNullProperties ignore these properties and the changes are not discarded + if (importedApplicationDetail != null && existingApplicationDetail != null) { + if (importedApplicationDetail.getAppPositioning() == null) { + existingApplicationDetail.setAppPositioning(null); + } + + if (importedApplicationDetail.getNavigationSetting() == null) { + existingApplicationDetail.setNavigationSetting(null); + } + } + } + public static void setPropertiesToExistingApplication( Application importedApplication, Application existingApplication) { importedApplication.setId(existingApplication.getId()); + + ApplicationDetail importedUnpublishedAppDetail = importedApplication.getUnpublishedApplicationDetail(); + ApplicationDetail importedPublishedAppDetail = importedApplication.getPublishedApplicationDetail(); + ApplicationDetail existingUnpublishedAppDetail = existingApplication.getUnpublishedApplicationDetail(); + ApplicationDetail existingPublishedAppDetail = existingApplication.getPublishedApplicationDetail(); + // For the existing application we don't need to default // value of the flag // The isPublic flag has a default value as false and this @@ -93,10 +121,10 @@ public class ImportExportUtils { // These properties are not present in the application when it is created, hence the initial commit // to git doesn't contain these keys and if we want to discard the changes, the function // copyNestedNonNullProperties ignore these properties and the changes are not discarded - if (importedApplication.getUnpublishedApplicationDetail() == null) { + if (importedUnpublishedAppDetail == null) { existingApplication.setUnpublishedApplicationDetail(null); } - if (importedApplication.getPublishedApplicationDetail() == null) { + if (importedPublishedAppDetail == null) { existingApplication.setPublishedApplicationDetail(null); } if (importedApplication.getPublishedAppLayout() == null) { @@ -106,6 +134,9 @@ public class ImportExportUtils { existingApplication.setUnpublishedAppLayout(null); } + setPropertiesToApplicationDetail(importedUnpublishedAppDetail, existingUnpublishedAppDetail); + setPropertiesToApplicationDetail(importedPublishedAppDetail, existingPublishedAppDetail); + copyNestedNonNullProperties(importedApplication, existingApplication); } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportApplicationServiceTests.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportApplicationServiceTests.java index e799141df3..49164f48a6 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportApplicationServiceTests.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportApplicationServiceTests.java @@ -2507,6 +2507,119 @@ public class ImportApplicationServiceTests { .verifyComplete(); } + /** + * Testcase for checking the discard changes flow for following events: + * 1. Import application in org which has app positioning in applicationDetail already added + * 2. Add Navigation Settings to the imported application + * 3. User tries to import application from same application json file + * 4. Added NavigationSetting will be removed + */ + @Test + @WithUserDetails(value = "api_user") + public void + discardChange_addNavigationSettingAfterAppPositioningAlreadyPresentInImport_addedNavigationSettingRemoved() { + Mono applicationJsonMono = + createAppJson("test_assets/ImportExportServiceTest/valid-application-with-app-positioning.json"); + String workspaceId = createTemplateWorkspace().getId(); + final Mono resultMonoWithoutDiscardOperation = applicationJsonMono + .flatMap(applicationJson -> { + applicationJson + .getExportedApplication() + .setName("discard-change-navsettings-added-appPositioning-present"); + return importApplicationService.importNewApplicationInWorkspaceFromJson( + workspaceId, applicationJson); + }) + .flatMap(application -> { + ApplicationDetail applicationDetail = application.getUnpublishedApplicationDetail(); + Application.NavigationSetting navigationSetting = new Application.NavigationSetting(); + navigationSetting.setOrientation("top"); + applicationDetail.setNavigationSetting(navigationSetting); + application.setUnpublishedApplicationDetail(applicationDetail); + application.setPublishedApplicationDetail(applicationDetail); + return applicationService.save(application); + }) + .cache(); + + StepVerifier.create(resultMonoWithoutDiscardOperation) + .assertNext(initialApplication -> { + assertThat(initialApplication.getUnpublishedApplicationDetail()) + .isNotNull(); + assertThat(initialApplication + .getUnpublishedApplicationDetail() + .getNavigationSetting()) + .isNotNull(); + assertThat(initialApplication + .getUnpublishedApplicationDetail() + .getNavigationSetting() + .getOrientation()) + .isEqualTo("top"); + assertThat(initialApplication.getPublishedApplicationDetail()) + .isNotNull(); + assertThat(initialApplication + .getPublishedApplicationDetail() + .getNavigationSetting()) + .isNotNull(); + assertThat(initialApplication + .getPublishedApplicationDetail() + .getNavigationSetting() + .getOrientation()) + .isEqualTo("top"); + assertThat(initialApplication + .getUnpublishedApplicationDetail() + .getAppPositioning()) + .isNotNull(); + assertThat(initialApplication + .getUnpublishedApplicationDetail() + .getAppPositioning() + .getType()) + .isEqualTo(Application.AppPositioning.Type.AUTO); + assertThat(initialApplication + .getPublishedApplicationDetail() + .getAppPositioning()) + .isNotNull(); + assertThat(initialApplication + .getPublishedApplicationDetail() + .getAppPositioning() + .getType()) + .isEqualTo(Application.AppPositioning.Type.AUTO); + }) + .verifyComplete(); + // Import the same application again + final Mono resultMonoWithDiscardOperation = resultMonoWithoutDiscardOperation.flatMap( + importedApplication -> applicationJsonMono.flatMap(applicationJson -> { + importedApplication.setGitApplicationMetadata(new GitApplicationMetadata()); + importedApplication + .getGitApplicationMetadata() + .setDefaultApplicationId(importedApplication.getId()); + return applicationService + .save(importedApplication) + .then(importApplicationService.importApplicationInWorkspaceFromGit( + importedApplication.getWorkspaceId(), + applicationJson, + importedApplication.getId(), + "main")); + })); + + StepVerifier.create(resultMonoWithDiscardOperation) + .assertNext(application -> { + assertThat(application.getWorkspaceId()).isNotNull(); + assertThat(application.getUnpublishedApplicationDetail()).isNotNull(); + assertThat(application.getPublishedApplicationDetail()).isNotNull(); + assertThat(application.getUnpublishedApplicationDetail().getAppPositioning()) + .isNotNull(); + assertThat(application + .getUnpublishedApplicationDetail() + .getAppPositioning() + .getType()) + .isEqualTo(Application.AppPositioning.Type.AUTO); + assertThat(application.getUnpublishedApplicationDetail().getNavigationSetting()) + .isNull(); + assertThat(application.getPublishedApplicationDetail().getNavigationSetting()) + .isNull(); + }) + .verifyComplete(); + } + @Test @WithUserDetails(value = "api_user") public void applySchemaMigration_jsonFileWithFirstVersion_migratedToLatestVersionSuccess() { diff --git a/app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-with-app-positioning.json b/app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-with-app-positioning.json new file mode 100644 index 0000000000..e678d988fb --- /dev/null +++ b/app/server/appsmith-server/src/test/resources/test_assets/ImportExportServiceTest/valid-application-with-app-positioning.json @@ -0,0 +1,148 @@ +{ + "exportedApplication": { + "name": "Application with ApplicationDetail - App Positioning", + "isPublic": false, + "pages": [ + { + "id": "Page1", + "isDefault": true + } + ], + "publishedPages": [ + { + "id": "Page1", + "isDefault": true + } + ], + "viewMode": false, + "appIsExample": false, + "unreadCommentThreads": 0.0, + "unpublishedApplicationDetail": { + "appPositioning": { + "type": "AUTO" + } + }, + "color": "#E3DEFF", + "icon": "lotus", + "slug": "untitled-application-1", + "unpublishedCustomJSLibs": [], + "publishedCustomJSLibs": [], + "evaluationVersion": 2.0, + "applicationVersion": 2.0, + "collapseInvisibleWidgets": true, + "isManualUpdate": false, + "deleted": false + }, + "datasourceList": [], + "customJSLibList": [], + "pageList": [ + { + "unpublishedPage": { + "name": "Page1", + "slug": "page1", + "layouts": [ + { + "viewMode": false, + "dsl": { + "widgetName": "MainContainer", + "backgroundColor": "none", + "rightColumn": 4896.0, + "snapColumns": 64.0, + "detachFromLayout": true, + "widgetId": "0", + "topRow": 0.0, + "bottomRow": 5000.0, + "containerStyle": "none", + "snapRows": 124.0, + "parentRowSpace": 1.0, + "type": "CANVAS_WIDGET", + "canExtend": true, + "version": 87.0, + "minHeight": 1292.0, + "useAutoLayout": true, + "dynamicTriggerPathList": [], + "parentColumnSpace": 1.0, + "responsiveBehavior": "fill", + "dynamicBindingPathList": [], + "leftColumn": 0.0, + "children": [], + "positioning": "vertical", + "flexLayers": [] + }, + "layoutOnLoadActions": [], + "layoutOnLoadActionErrors": [], + "validOnPageLoadActions": true, + "id": "Page1", + "deleted": false, + "policies": [], + "userPermissions": [] + } + ], + "userPermissions": [], + "policies": [] + }, + "publishedPage": { + "name": "Page1", + "slug": "page1", + "layouts": [ + { + "viewMode": false, + "dsl": { + "widgetName": "MainContainer", + "backgroundColor": "none", + "rightColumn": 1224.0, + "snapColumns": 16.0, + "detachFromLayout": true, + "widgetId": "0", + "topRow": 0.0, + "bottomRow": 1250.0, + "containerStyle": "none", + "snapRows": 33.0, + "parentRowSpace": 1.0, + "type": "CANVAS_WIDGET", + "canExtend": true, + "version": 4.0, + "minHeight": 1292.0, + "dynamicTriggerPathList": [], + "parentColumnSpace": 1.0, + "dynamicBindingPathList": [], + "leftColumn": 0.0, + "children": [] + }, + "validOnPageLoadActions": true, + "id": "Page1", + "deleted": false, + "policies": [], + "userPermissions": [] + } + ], + "userPermissions": [], + "policies": [] + }, + "deleted": false, + "gitSyncId": "65367d79e575c11b12c0ec77_65367d79e575c11b12c0ec79" + } + ], + "actionList": [], + "actionCollectionList": [], + "updatedResources": { + "customJSLibList": [], + "actionList": [], + "pageList": [ + "Page1" + ], + "actionCollectionList": [] + }, + "editModeTheme": { + "name": "Default-New", + "displayName": "Modern", + "isSystemTheme": true, + "deleted": false + }, + "publishedTheme": { + "name": "Default-New", + "displayName": "Modern", + "isSystemTheme": true, + "deleted": false + } +}