From 6ce1e35ed18a292734ccf85f07475c91344aec15 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Mon, 7 Jun 2021 15:55:52 +0530 Subject: [PATCH 01/18] Add API for setting page order in an application --- .../controllers/ApplicationController.java | 14 +++++++++++-- .../server/domains/ApplicationPage.java | 2 ++ .../CustomApplicationRepository.java | 4 ++++ .../CustomApplicationRepositoryImpl.java | 11 +++++++++- .../services/ApplicationPageService.java | 2 ++ .../services/ApplicationPageServiceImpl.java | 21 +++++++++++++++++++ 6 files changed, 51 insertions(+), 3 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ApplicationController.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ApplicationController.java index a1d7fca78d..074d082415 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ApplicationController.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ApplicationController.java @@ -87,6 +87,16 @@ public class ApplicationController extends BaseController new ResponseDTO<>(HttpStatus.OK.value(), updatedApplication, null)); } + @PutMapping("/{applicationId}/page/{pageId}/reorder") + public Mono> reorderPage( + @PathVariable String applicationId, + @PathVariable String pageId, + @RequestParam Integer order + ) { + return applicationPageService.reorderPage(applicationId, pageId, order) + .map(updatedApplication -> new ResponseDTO<>(HttpStatus.OK.value(), updatedApplication, null)); + } + @DeleteMapping("/{id}") public Mono> delete(@PathVariable String id) { log.debug("Going to delete application with id: {}", id); @@ -132,7 +142,7 @@ public class ApplicationController extends BaseController> getApplicationFile(@PathVariable String id) { log.debug("Going to export application with id: {}", id); - + return importExportApplicationService.exportApplicationById(id) .map(fetchedResource -> { String applicationName = fetchedResource.getExportedApplication().getName(); @@ -143,7 +153,7 @@ public class ApplicationController extends BaseController { @@ -22,6 +24,8 @@ public interface CustomApplicationRepository extends AppsmithRepository addPageToApplication(String applicationId, String pageId, boolean isDefault); + Mono setPages(String applicationId, List pages); + Mono setDefaultPage(String applicationId, String pageId); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java index c89afbed96..8b6e0b71d8 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java @@ -77,7 +77,16 @@ public class CustomApplicationRepositoryImpl extends BaseAppsmithRepositoryImpl< final ApplicationPage applicationPage = new ApplicationPage(pageId, isDefault); return mongoOperations.updateFirst( Query.query(getIdCriteria(applicationId)), - new Update().addToSet(FieldName.PAGES, applicationPage), + new Update().addToSet(fieldName(QApplication.application.pages), applicationPage), + Application.class + ); + } + + @Override + public Mono setPages(String applicationId, List pages) { + return mongoOperations.updateFirst( + Query.query(getIdCriteria(applicationId)), + new Update().set(fieldName(QApplication.application.pages), pages), Application.class ); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java index 2b353ccd0f..71f81b9055 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java @@ -36,4 +36,6 @@ public interface ApplicationPageService { Mono publish(String applicationId); void generateAndSetPagePolicies(Application application, PageDTO page); + + Mono reorderPage(String applicationId, String pageId, Integer order); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java index 0cfa0195da..ecc3176edb 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java @@ -667,4 +667,25 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { }); } + @Override + public Mono reorderPage(String applicationId, String pageId, Integer order) { + return applicationService.findById(applicationId, MANAGE_APPLICATIONS) + .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.ACL_NO_RESOURCE_FOUND, FieldName.APPLICATION, applicationId))) + .flatMap(application -> { + // Update the order in unpublished pages here, since this should only ever happen in edit mode. + final List pages = application.getPages(); + + ApplicationPage foundPage = null; + for (final ApplicationPage page : pages) { + if (pageId.equals(page.getId())) { + foundPage = page; + } + } + + return applicationRepository + .setPages(applicationId, pages) + .then(applicationService.getById(applicationId)); + }); + } + } From 4787b749581b1b170c7a4b745ff30067308649c3 Mon Sep 17 00:00:00 2001 From: Shrikant Sharat Kandula Date: Mon, 7 Jun 2021 15:55:52 +0530 Subject: [PATCH 02/18] Add API for setting page order in an application --- .../controllers/ApplicationController.java | 14 +++++++++++-- .../server/domains/ApplicationPage.java | 2 ++ .../CustomApplicationRepository.java | 4 ++++ .../CustomApplicationRepositoryImpl.java | 11 +++++++++- .../services/ApplicationPageService.java | 2 ++ .../services/ApplicationPageServiceImpl.java | 21 +++++++++++++++++++ 6 files changed, 51 insertions(+), 3 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ApplicationController.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ApplicationController.java index a1d7fca78d..074d082415 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ApplicationController.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ApplicationController.java @@ -87,6 +87,16 @@ public class ApplicationController extends BaseController new ResponseDTO<>(HttpStatus.OK.value(), updatedApplication, null)); } + @PutMapping("/{applicationId}/page/{pageId}/reorder") + public Mono> reorderPage( + @PathVariable String applicationId, + @PathVariable String pageId, + @RequestParam Integer order + ) { + return applicationPageService.reorderPage(applicationId, pageId, order) + .map(updatedApplication -> new ResponseDTO<>(HttpStatus.OK.value(), updatedApplication, null)); + } + @DeleteMapping("/{id}") public Mono> delete(@PathVariable String id) { log.debug("Going to delete application with id: {}", id); @@ -132,7 +142,7 @@ public class ApplicationController extends BaseController> getApplicationFile(@PathVariable String id) { log.debug("Going to export application with id: {}", id); - + return importExportApplicationService.exportApplicationById(id) .map(fetchedResource -> { String applicationName = fetchedResource.getExportedApplication().getName(); @@ -143,7 +153,7 @@ public class ApplicationController extends BaseController { @@ -22,6 +24,8 @@ public interface CustomApplicationRepository extends AppsmithRepository addPageToApplication(String applicationId, String pageId, boolean isDefault); + Mono setPages(String applicationId, List pages); + Mono setDefaultPage(String applicationId, String pageId); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java index c89afbed96..8b6e0b71d8 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java @@ -77,7 +77,16 @@ public class CustomApplicationRepositoryImpl extends BaseAppsmithRepositoryImpl< final ApplicationPage applicationPage = new ApplicationPage(pageId, isDefault); return mongoOperations.updateFirst( Query.query(getIdCriteria(applicationId)), - new Update().addToSet(FieldName.PAGES, applicationPage), + new Update().addToSet(fieldName(QApplication.application.pages), applicationPage), + Application.class + ); + } + + @Override + public Mono setPages(String applicationId, List pages) { + return mongoOperations.updateFirst( + Query.query(getIdCriteria(applicationId)), + new Update().set(fieldName(QApplication.application.pages), pages), Application.class ); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java index 2b353ccd0f..71f81b9055 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java @@ -36,4 +36,6 @@ public interface ApplicationPageService { Mono publish(String applicationId); void generateAndSetPagePolicies(Application application, PageDTO page); + + Mono reorderPage(String applicationId, String pageId, Integer order); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java index 0cfa0195da..ecc3176edb 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java @@ -667,4 +667,25 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { }); } + @Override + public Mono reorderPage(String applicationId, String pageId, Integer order) { + return applicationService.findById(applicationId, MANAGE_APPLICATIONS) + .switchIfEmpty(Mono.error(new AppsmithException(AppsmithError.ACL_NO_RESOURCE_FOUND, FieldName.APPLICATION, applicationId))) + .flatMap(application -> { + // Update the order in unpublished pages here, since this should only ever happen in edit mode. + final List pages = application.getPages(); + + ApplicationPage foundPage = null; + for (final ApplicationPage page : pages) { + if (pageId.equals(page.getId())) { + foundPage = page; + } + } + + return applicationRepository + .setPages(applicationId, pages) + .then(applicationService.getById(applicationId)); + }); + } + } From 7acba5e1df17546cc19881131d497d74255f4f85 Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Thu, 10 Jun 2021 12:12:48 +0530 Subject: [PATCH 03/18] Add page re-order API --- .../server/domains/ApplicationPage.java | 5 +++++ .../services/ApplicationPageServiceImpl.java | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java index e6ab71aae1..8dd39116a8 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java @@ -27,4 +27,9 @@ public class ApplicationPage { return Boolean.TRUE.equals(isDefault); } + public ApplicationPage(String id, boolean isDefault){ + this.id = id; + this.isDefault = isDefault; + } + } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java index ecc3176edb..6fe380f694 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java @@ -681,7 +681,24 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { foundPage = page; } } + //Set the order for the pages which has null values. At present we dont store the order and hence we need to update the values + for (int i = 1; i <= pages.size(); i++) { + ApplicationPage page = pages.get(i - 1); + if (page.getOrder() == null) { + page.setOrder(i); + } + } + if(foundPage != null){ + //Increment the order of subsequent pages + for (final ApplicationPage page : pages) { + if (page.getOrder() < foundPage.getOrder()) { + page.setOrder(page.getOrder()+1); + } + } + //set the selected page order to 0 + foundPage.setOrder(0); + } return applicationRepository .setPages(applicationId, pages) .then(applicationService.getById(applicationId)); From bcd5f0600dab28817ea24a78fb1e54a058704cac Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Thu, 10 Jun 2021 12:23:47 +0530 Subject: [PATCH 04/18] Add page re-order API --- .../com/appsmith/server/services/ApplicationPageServiceImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java index 6fe380f694..bfa3a523d7 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java @@ -696,6 +696,7 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { page.setOrder(page.getOrder()+1); } } + //set the selected page order to 0 foundPage.setOrder(0); } From fea54e2fa02efdc7e76829b5d7ea684dffa37021 Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Thu, 10 Jun 2021 17:12:29 +0530 Subject: [PATCH 05/18] Moved constructor to the top. --- .../com/appsmith/server/domains/ApplicationPage.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java index 8dd39116a8..452cefe7b3 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java @@ -22,14 +22,14 @@ public class ApplicationPage { Integer order; - @JsonIgnore - public boolean isDefault() { - return Boolean.TRUE.equals(isDefault); - } - public ApplicationPage(String id, boolean isDefault){ this.id = id; this.isDefault = isDefault; } + @JsonIgnore + public boolean isDefault() { + return Boolean.TRUE.equals(isDefault); + } + } From ccd04b14d9ef264f7e00dcc1fb4a01c4d3759f89 Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Mon, 14 Jun 2021 10:19:02 +0530 Subject: [PATCH 06/18] 1. Added logic to handle pageDown scenario 2. Added comments --- .../services/ApplicationPageServiceImpl.java | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java index bfa3a523d7..d4e21597a9 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java @@ -667,6 +667,13 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { }); } + /** This function walks thorugh all the pages and reorders them and updates the order as per the user preference. + * A page can be moved up or down from the current position and accordingly the order of the remaining page changes. + * @param applicationId The id of the Application + * @param pageId Targetted page id + * @param order New order for the selected page + * @return Application object with the latest order + **/ @Override public Mono reorderPage(String applicationId, String pageId, Integer order) { return applicationService.findById(applicationId, MANAGE_APPLICATIONS) @@ -689,16 +696,28 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { } } + /* there are two cases where page is re-ordered. Lets assume there are five pages 1,2,3,4,5 + * Case 1(isMovingUp == true): p5 to p2, order of p2,p3,p4 increases by 1. + * + * Case 2(isMovingUp == false): p2 to p5, order of p3,p4,p5 decreases by 1. + **/ if(foundPage != null){ - //Increment the order of subsequent pages - for (final ApplicationPage page : pages) { - if (page.getOrder() < foundPage.getOrder()) { - page.setOrder(page.getOrder()+1); + boolean isMovingUp = order > foundPage.getOrder(); + if(isMovingUp){ + for (final ApplicationPage page : pages) { + if (page.getOrder() < foundPage.getOrder() && page.getOrder() >= order) { + page.setOrder(page.getOrder()+1); + } + } + } else{ + for (final ApplicationPage page : pages) { + if (page.getOrder() >= foundPage.getOrder() && page.getOrder() < order) { + page.setOrder(page.getOrder()-1); + } } } - //set the selected page order to 0 - foundPage.setOrder(0); + foundPage.setOrder(order); } return applicationRepository .setPages(applicationId, pages) From 1ca7a0cb206d3021118dea1e26d012a72d671923 Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Mon, 14 Jun 2021 15:34:07 +0530 Subject: [PATCH 07/18] Add test for page reorder and add order to the response --- .../com/appsmith/server/dtos/PageDTO.java | 2 + .../server/services/PageServiceTest.java | 109 +++++++++++++++++- 2 files changed, 105 insertions(+), 6 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java index 3931091825..81bda58653 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java @@ -41,4 +41,6 @@ public class PageDTO { Boolean isHidden; + Integer order; + } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java index da49dcf673..9388c46b79 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java @@ -4,12 +4,7 @@ import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.Policy; import com.appsmith.external.plugins.PluginExecutor; import com.appsmith.server.constants.FieldName; -import com.appsmith.server.domains.Application; -import com.appsmith.server.domains.Datasource; -import com.appsmith.server.domains.Layout; -import com.appsmith.server.domains.NewAction; -import com.appsmith.server.domains.Plugin; -import com.appsmith.server.domains.User; +import com.appsmith.server.domains.*; import com.appsmith.server.dtos.ActionDTO; import com.appsmith.server.dtos.LayoutDTO; import com.appsmith.server.dtos.PageDTO; @@ -49,6 +44,7 @@ import static com.appsmith.server.acl.AclPermission.MANAGE_PAGES; import static com.appsmith.server.acl.AclPermission.READ_ACTIONS; import static com.appsmith.server.acl.AclPermission.READ_PAGES; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertTrue; @RunWith(SpringRunner.class) @SpringBootTest @@ -154,6 +150,7 @@ public class PageServiceTest { testPage.setName("PageServiceTest TestApp"); setupTestApplication(); testPage.setApplicationId(application.getId()); + testPage.setOrder(0); Mono pageMono = applicationPageService.createPage(testPage); @@ -171,6 +168,7 @@ public class PageServiceTest { assertThat(page.getLayouts()).isNotEmpty(); assertThat(page.getLayouts().get(0).getDsl()).isEqualTo(parsedJson); assertThat(page.getLayouts().get(0).getWidgetNames()).isNotEmpty(); + assertTrue(page.getOrder() == 0); }) .verifyComplete(); } @@ -187,6 +185,7 @@ public class PageServiceTest { PageDTO testPage = new PageDTO(); testPage.setName("PageServiceTest TestApp"); + testPage.setOrder(1); setupTestApplication(); testPage.setApplicationId(application.getId()); @@ -209,6 +208,7 @@ public class PageServiceTest { assertThat(page.getLayouts()).isNotEmpty(); assertThat(page.getLayouts().get(0).getDsl()).isEqualTo(dsl); + assertTrue(page.getOrder() == 1); }) .verifyComplete(); } @@ -385,6 +385,103 @@ public class PageServiceTest { .verifyComplete(); } + @Test + @WithUserDetails(value = "api_user") + public void reOrderPageMovingUp() { + + PageDTO testPage = new PageDTO(); + testPage.setName("PageReorder TestApp1"); + setupTestApplication(); + testPage.setApplicationId(application.getId()); + testPage.setOrder(0); + String pageId1 = testPage.getId(); + + applicationPageService.createPage(testPage); + + testPage = new PageDTO(); + testPage.setName("PageReorder TestApp2"); + testPage.setApplicationId(application.getId()); + testPage.setOrder(1); + String pageId2 = testPage.getId(); + + applicationPageService.createPage(testPage); + + testPage = new PageDTO(); + testPage.setName("PageReorder TestApp3"); + testPage.setApplicationId(application.getId()); + testPage.setOrder(2); + String pageId3 = testPage.getId(); + + applicationPageService.createPage(testPage); + + Mono applicationMono = applicationPageService.reorderPage(application.getId(), pageId3, 0); + StepVerifier + .create(applicationMono) + .assertNext(application ->{ + final List pages = application.getPages(); + for(ApplicationPage page : pages){ + if(pageId3.equals(page.getId())){ + assertTrue(page.getOrder() == 0); + } + if(pageId2.equals(page.getId())){ + assertTrue(page.getOrder() == 2); + } + if(pageId1.equals(page.getId())){ + assertTrue(page.getOrder() == 1); + } + } + } ) + .verifyComplete(); + } + + @Test + @WithUserDetails(value ="aps_user") + public void reOrderPageMovingDown() { + + PageDTO testPage = new PageDTO(); + testPage.setName("PageReorder TestApp1"); + setupTestApplication(); + testPage.setApplicationId(application.getId()); + testPage.setOrder(0); + String pageId1 = testPage.getId(); + + applicationPageService.createPage(testPage); + + testPage = new PageDTO(); + testPage.setName("PageReorder TestApp2"); + testPage.setApplicationId(application.getId()); + testPage.setOrder(1); + String pageId2 = testPage.getId(); + + applicationPageService.createPage(testPage); + + testPage = new PageDTO(); + testPage.setName("PageReorder TestApp3"); + testPage.setApplicationId(application.getId()); + testPage.setOrder(2); + String pageId3 = testPage.getId(); + + applicationPageService.createPage(testPage); + + Mono applicationMono = applicationPageService.reorderPage(application.getId(), pageId1, 2); + StepVerifier + .create(applicationMono) + .assertNext(application ->{ + final List pages = application.getPages(); + for(ApplicationPage page : pages){ + if(pageId3.equals(page.getId())){ + assertTrue(page.getOrder() == 1); + } + if(pageId2.equals(page.getId())){ + assertTrue(page.getOrder() == 0); + } + if(pageId1.equals(page.getId())){ + assertTrue(page.getOrder() == 2); + } + } + } ) + .verifyComplete(); + } @After public void purgeAllPages() { From 9932f918f9471f0799e5a74b1a41c8c4369922ed Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Tue, 15 Jun 2021 10:03:39 +0530 Subject: [PATCH 08/18] Use assertThat type of assertions and move the order initialization logic to migration --- .../services/ApplicationPageServiceImpl.java | 11 ++--------- .../server/services/PageServiceTest.java | 17 ++++++++--------- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java index d4e21597a9..bad4f0c413 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java @@ -667,7 +667,7 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { }); } - /** This function walks thorugh all the pages and reorders them and updates the order as per the user preference. + /** This function walks through all the pages and reorders them and updates the order as per the user preference. * A page can be moved up or down from the current position and accordingly the order of the remaining page changes. * @param applicationId The id of the Application * @param pageId Targetted page id @@ -688,13 +688,6 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { foundPage = page; } } - //Set the order for the pages which has null values. At present we dont store the order and hence we need to update the values - for (int i = 1; i <= pages.size(); i++) { - ApplicationPage page = pages.get(i - 1); - if (page.getOrder() == null) { - page.setOrder(i); - } - } /* there are two cases where page is re-ordered. Lets assume there are five pages 1,2,3,4,5 * Case 1(isMovingUp == true): p5 to p2, order of p2,p3,p4 increases by 1. @@ -716,7 +709,7 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { } } } - //set the selected page order to 0 + //set the selected page order to the given order foundPage.setOrder(order); } return applicationRepository diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java index 9388c46b79..7a1d1411f6 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java @@ -44,7 +44,6 @@ import static com.appsmith.server.acl.AclPermission.MANAGE_PAGES; import static com.appsmith.server.acl.AclPermission.READ_ACTIONS; import static com.appsmith.server.acl.AclPermission.READ_PAGES; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.assertTrue; @RunWith(SpringRunner.class) @SpringBootTest @@ -168,7 +167,7 @@ public class PageServiceTest { assertThat(page.getLayouts()).isNotEmpty(); assertThat(page.getLayouts().get(0).getDsl()).isEqualTo(parsedJson); assertThat(page.getLayouts().get(0).getWidgetNames()).isNotEmpty(); - assertTrue(page.getOrder() == 0); + assertThat(page.getOrder()).isEqualTo(0); }) .verifyComplete(); } @@ -208,7 +207,7 @@ public class PageServiceTest { assertThat(page.getLayouts()).isNotEmpty(); assertThat(page.getLayouts().get(0).getDsl()).isEqualTo(dsl); - assertTrue(page.getOrder() == 1); + assertThat(page.getOrder()).isEqualTo(1); }) .verifyComplete(); } @@ -421,13 +420,13 @@ public class PageServiceTest { final List pages = application.getPages(); for(ApplicationPage page : pages){ if(pageId3.equals(page.getId())){ - assertTrue(page.getOrder() == 0); + assertThat(page.getOrder()).isEqualTo(0); } if(pageId2.equals(page.getId())){ - assertTrue(page.getOrder() == 2); + assertThat(page.getOrder()).isEqualTo(2); } if(pageId1.equals(page.getId())){ - assertTrue(page.getOrder() == 1); + assertThat(page.getOrder()).isEqualTo(1); } } } ) @@ -470,13 +469,13 @@ public class PageServiceTest { final List pages = application.getPages(); for(ApplicationPage page : pages){ if(pageId3.equals(page.getId())){ - assertTrue(page.getOrder() == 1); + assertThat(page.getOrder()).isEqualTo(1); } if(pageId2.equals(page.getId())){ - assertTrue(page.getOrder() == 0); + assertThat(page.getOrder()).isEqualTo(0); } if(pageId1.equals(page.getId())){ - assertTrue(page.getOrder() == 2); + assertThat(page.getOrder()).isEqualTo(2); } } } ) From 3e1702b4f9bcb55258a3de990d670287add4730c Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Wed, 16 Jun 2021 09:12:53 +0530 Subject: [PATCH 09/18] 1. Add migration logic 2. Add order while creating pages --- .../appsmith/server/constants/FieldName.java | 1 + .../server/domains/ApplicationPage.java | 5 -- .../com/appsmith/server/dtos/PageDTO.java | 2 - .../server/migrations/DatabaseChangelog.java | 60 ++++++++++--------- .../CustomApplicationRepository.java | 2 +- .../CustomApplicationRepositoryImpl.java | 4 +- .../services/ApplicationPageServiceImpl.java | 3 +- .../server/services/PageServiceTest.java | 43 +++++++------ 8 files changed, 59 insertions(+), 61 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/FieldName.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/FieldName.java index 3b044a72c4..51ad052619 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/FieldName.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/FieldName.java @@ -71,4 +71,5 @@ public class FieldName { public static String MONGO_ESCAPE_CLASS = "appsmith_mongo_escape_class"; public static String MONGO_UNESCAPED_ID = "_id"; public static String MONGO_UNESCAPED_CLASS = "_class"; + public static Integer DEFAULT_PAGE_ORDER = 0; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java index 452cefe7b3..e6ab71aae1 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java @@ -22,11 +22,6 @@ public class ApplicationPage { Integer order; - public ApplicationPage(String id, boolean isDefault){ - this.id = id; - this.isDefault = isDefault; - } - @JsonIgnore public boolean isDefault() { return Boolean.TRUE.equals(isDefault); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java index 81bda58653..3931091825 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/PageDTO.java @@ -41,6 +41,4 @@ public class PageDTO { Boolean isHidden; - Integer order; - } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java index ae884e8c3e..bc55f6a8b4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java @@ -14,34 +14,7 @@ import com.appsmith.server.acl.AclPermission; import com.appsmith.server.acl.AppsmithRole; import com.appsmith.server.constants.Appsmith; import com.appsmith.server.constants.FieldName; -import com.appsmith.server.domains.Action; -import com.appsmith.server.domains.Application; -import com.appsmith.server.domains.Collection; -import com.appsmith.server.domains.Config; -import com.appsmith.server.domains.Datasource; -import com.appsmith.server.domains.Group; -import com.appsmith.server.domains.InviteUser; -import com.appsmith.server.domains.Layout; -import com.appsmith.server.domains.NewAction; -import com.appsmith.server.domains.NewPage; -import com.appsmith.server.domains.Organization; -import com.appsmith.server.domains.OrganizationPlugin; -import com.appsmith.server.domains.Page; -import com.appsmith.server.domains.PasswordResetToken; -import com.appsmith.server.domains.Permission; -import com.appsmith.server.domains.Plugin; -import com.appsmith.server.domains.PluginType; -import com.appsmith.server.domains.QApplication; -import com.appsmith.server.domains.QConfig; -import com.appsmith.server.domains.QDatasource; -import com.appsmith.server.domains.QNewAction; -import com.appsmith.server.domains.QOrganization; -import com.appsmith.server.domains.QPlugin; -import com.appsmith.server.domains.Role; -import com.appsmith.server.domains.Sequence; -import com.appsmith.server.domains.User; -import com.appsmith.server.domains.UserData; -import com.appsmith.server.domains.UserRole; +import com.appsmith.server.domains.*; import com.appsmith.server.dtos.ActionDTO; import com.appsmith.server.dtos.DslActionDTO; import com.appsmith.server.dtos.OrganizationPluginStatus; @@ -2386,4 +2359,35 @@ public class DatabaseChangelog { firestoreActionQueries.stream() .forEach(action -> mongoTemplate.save(action)); } + + /** + * - Older order file where not present for the pages created within the application because page reordering with in + * the application was not supported. + * - New Form order field will be added to the Page object and is used to order the pages with in the application + * Since the previously created pages doesnt have the order, we will be updating/adding order to all the previously + * created pages of all the application present. + * - [] + */ + @ChangeSet(order = "071", id = "add-and-update-order-for-all-pages", author = "") + public void addOrderToAllPagesOfApplication(MongoTemplate mongoTemplate) { + for (Application application : mongoTemplate.findAll(Application.class)) { + if(application.getPages() != null) { + int i = 0; + for (ApplicationPage page : application.getPages()) { + page.setOrder(i); + i++; + } + if(application.getPublishedPages() != null) { + for (ApplicationPage page : application.getPublishedPages()) { + for(ApplicationPage npage: application.getPages()) { + if(npage.getId().equals(page.getId())) { + page.setOrder(npage.getOrder()); + } + } + } + } + mongoTemplate.save(application); + } + } + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepository.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepository.java index afe5bdbe66..80d1aa0761 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepository.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepository.java @@ -22,7 +22,7 @@ public interface CustomApplicationRepository extends AppsmithRepository findByClonedFromApplicationId(String applicationId, AclPermission permission); - Mono addPageToApplication(String applicationId, String pageId, boolean isDefault); + Mono addPageToApplication(String applicationId, String pageId, boolean isDefault, Integer order); Mono setPages(String applicationId, List pages); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java index 8b6e0b71d8..93ed1e9f84 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java @@ -73,8 +73,8 @@ public class CustomApplicationRepositoryImpl extends BaseAppsmithRepositoryImpl< } @Override - public Mono addPageToApplication(String applicationId, String pageId, boolean isDefault) { - final ApplicationPage applicationPage = new ApplicationPage(pageId, isDefault); + public Mono addPageToApplication(String applicationId, String pageId, boolean isDefault, Integer order) { + final ApplicationPage applicationPage = new ApplicationPage(pageId, isDefault, order); return mongoOperations.updateFirst( Query.query(getIdCriteria(applicationId)), new Update().addToSet(fieldName(QApplication.application.pages), applicationPage), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java index bad4f0c413..c66dcc7409 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java @@ -139,7 +139,8 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { */ @Override public Mono addPageToApplication(Application application, PageDTO page, Boolean isDefault) { - return applicationRepository.addPageToApplication(application.getId(), page.getId(), isDefault) + Integer order = application.getPages() != null ? application.getPages().size() : FieldName.DEFAULT_PAGE_ORDER; + return applicationRepository.addPageToApplication(application.getId(), page.getId(), isDefault, order) .doOnSuccess(result -> { if (result.getModifiedCount() != 1) { log.error("Add page to application didn't update anything, probably because application wasn't found."); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java index 7a1d1411f6..cc265d1551 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java @@ -149,7 +149,6 @@ public class PageServiceTest { testPage.setName("PageServiceTest TestApp"); setupTestApplication(); testPage.setApplicationId(application.getId()); - testPage.setOrder(0); Mono pageMono = applicationPageService.createPage(testPage); @@ -167,7 +166,6 @@ public class PageServiceTest { assertThat(page.getLayouts()).isNotEmpty(); assertThat(page.getLayouts().get(0).getDsl()).isEqualTo(parsedJson); assertThat(page.getLayouts().get(0).getWidgetNames()).isNotEmpty(); - assertThat(page.getOrder()).isEqualTo(0); }) .verifyComplete(); } @@ -184,7 +182,6 @@ public class PageServiceTest { PageDTO testPage = new PageDTO(); testPage.setName("PageServiceTest TestApp"); - testPage.setOrder(1); setupTestApplication(); testPage.setApplicationId(application.getId()); @@ -207,7 +204,6 @@ public class PageServiceTest { assertThat(page.getLayouts()).isNotEmpty(); assertThat(page.getLayouts().get(0).getDsl()).isEqualTo(dsl); - assertThat(page.getOrder()).isEqualTo(1); }) .verifyComplete(); } @@ -392,7 +388,6 @@ public class PageServiceTest { testPage.setName("PageReorder TestApp1"); setupTestApplication(); testPage.setApplicationId(application.getId()); - testPage.setOrder(0); String pageId1 = testPage.getId(); applicationPageService.createPage(testPage); @@ -400,7 +395,6 @@ public class PageServiceTest { testPage = new PageDTO(); testPage.setName("PageReorder TestApp2"); testPage.setApplicationId(application.getId()); - testPage.setOrder(1); String pageId2 = testPage.getId(); applicationPageService.createPage(testPage); @@ -408,7 +402,6 @@ public class PageServiceTest { testPage = new PageDTO(); testPage.setName("PageReorder TestApp3"); testPage.setApplicationId(application.getId()); - testPage.setOrder(2); String pageId3 = testPage.getId(); applicationPageService.createPage(testPage); @@ -434,33 +427,39 @@ public class PageServiceTest { } @Test - @WithUserDetails(value ="aps_user") + @WithUserDetails(value ="api_user") public void reOrderPageMovingDown() { PageDTO testPage = new PageDTO(); testPage.setName("PageReorder TestApp1"); setupTestApplication(); testPage.setApplicationId(application.getId()); - testPage.setOrder(0); + testPage.setId("a"); String pageId1 = testPage.getId(); - applicationPageService.createPage(testPage); + Mono pageDTO = applicationPageService.createPage(testPage); + StepVerifier.create(pageDTO); + applicationPageService.publish(application.getId()); - testPage = new PageDTO(); - testPage.setName("PageReorder TestApp2"); - testPage.setApplicationId(application.getId()); - testPage.setOrder(1); - String pageId2 = testPage.getId(); + PageDTO testPage1 = new PageDTO(); + testPage1.setName("PageReorder TestApp2"); + testPage1.setApplicationId(application.getId()); + testPage1.setId("b"); + String pageId2 = testPage1.getId(); - applicationPageService.createPage(testPage); + pageDTO = applicationPageService.createPage(testPage1); + StepVerifier.create(pageDTO); + applicationPageService.publish(application.getId()); - testPage = new PageDTO(); - testPage.setName("PageReorder TestApp3"); - testPage.setApplicationId(application.getId()); - testPage.setOrder(2); - String pageId3 = testPage.getId(); + PageDTO testPage2 = new PageDTO(); + testPage2.setName("PageReorder TestApp3"); + testPage2.setApplicationId(application.getId()); + testPage.setId("c"); + String pageId3 = testPage2.getId(); - applicationPageService.createPage(testPage); + pageDTO = applicationPageService.createPage(testPage2); + StepVerifier.create(pageDTO); + applicationPageService.publish(application.getId()); Mono applicationMono = applicationPageService.reorderPage(application.getId(), pageId1, 2); StepVerifier From 5a8ac3fd33306cfd5883ad6ecac1c1a7bbffbe41 Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Wed, 16 Jun 2021 11:21:19 +0530 Subject: [PATCH 10/18] 1. Fix failing unit tests for the page reorder --- .../server/services/PageServiceTest.java | 138 +++++++++--------- 1 file changed, 72 insertions(+), 66 deletions(-) diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java index cc265d1551..4b23a1e6a3 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java @@ -39,6 +39,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; import static com.appsmith.server.acl.AclPermission.MANAGE_PAGES; import static com.appsmith.server.acl.AclPermission.READ_ACTIONS; @@ -384,43 +385,50 @@ public class PageServiceTest { @WithUserDetails(value = "api_user") public void reOrderPageMovingUp() { - PageDTO testPage = new PageDTO(); - testPage.setName("PageReorder TestApp1"); + Application app = new Application(); + app.setName("validGetApplicationPagesMultiPageApp-Test"); setupTestApplication(); - testPage.setApplicationId(application.getId()); - String pageId1 = testPage.getId(); - applicationPageService.createPage(testPage); + Mono createApplicationMono = applicationPageService.createApplication(app, orgId) + .cache(); - testPage = new PageDTO(); - testPage.setName("PageReorder TestApp2"); - testPage.setApplicationId(application.getId()); - String pageId2 = testPage.getId(); - - applicationPageService.createPage(testPage); - - testPage = new PageDTO(); - testPage.setName("PageReorder TestApp3"); - testPage.setApplicationId(application.getId()); - String pageId3 = testPage.getId(); - - applicationPageService.createPage(testPage); - - Mono applicationMono = applicationPageService.reorderPage(application.getId(), pageId3, 0); + // Create all the pages for this application in a blocking manner. + createApplicationMono + .flatMap(application -> { + PageDTO testPage = new PageDTO(); + testPage.setName("Page2"); + testPage.setApplicationId(application.getId()); + return applicationPageService.createPage(testPage) + .then(Mono.just(application)); + }) + .flatMap(application -> { + PageDTO testPage = new PageDTO(); + testPage.setName("Page3"); + testPage.setApplicationId(application.getId()); + return applicationPageService.createPage(testPage) + .then(Mono.just(application)); + }) + .flatMap(application -> { + PageDTO testPage = new PageDTO(); + testPage.setName("Page4"); + testPage.setApplicationId(application.getId()); + return applicationPageService.createPage(testPage); + }) + .block(); + String pageId = application.getPages().get(3).getId(); + Mono applicationM = createApplicationMono. + flatMap( application -> { + return applicationPageService.reorderPage(application.getId(), application.getPages().get(3).getId(), 0); + }); StepVerifier - .create(applicationMono) + .create(applicationM) .assertNext(application ->{ final List pages = application.getPages(); + assertThat(application.getPages().size()).isEqualTo(4); for(ApplicationPage page : pages){ - if(pageId3.equals(page.getId())){ + if(pageId.equals(page.getId())){ assertThat(page.getOrder()).isEqualTo(0); } - if(pageId2.equals(page.getId())){ - assertThat(page.getOrder()).isEqualTo(2); - } - if(pageId1.equals(page.getId())){ - assertThat(page.getOrder()).isEqualTo(1); - } } } ) .verifyComplete(); @@ -430,51 +438,49 @@ public class PageServiceTest { @WithUserDetails(value ="api_user") public void reOrderPageMovingDown() { - PageDTO testPage = new PageDTO(); - testPage.setName("PageReorder TestApp1"); + Application app = new Application(); + app.setName("validGetApplicationPagesMultiPageApp-Test"); setupTestApplication(); - testPage.setApplicationId(application.getId()); - testPage.setId("a"); - String pageId1 = testPage.getId(); - Mono pageDTO = applicationPageService.createPage(testPage); - StepVerifier.create(pageDTO); - applicationPageService.publish(application.getId()); + Mono createApplicationMono = applicationPageService.createApplication(app, orgId) + .cache(); - PageDTO testPage1 = new PageDTO(); - testPage1.setName("PageReorder TestApp2"); - testPage1.setApplicationId(application.getId()); - testPage1.setId("b"); - String pageId2 = testPage1.getId(); - - pageDTO = applicationPageService.createPage(testPage1); - StepVerifier.create(pageDTO); - applicationPageService.publish(application.getId()); - - PageDTO testPage2 = new PageDTO(); - testPage2.setName("PageReorder TestApp3"); - testPage2.setApplicationId(application.getId()); - testPage.setId("c"); - String pageId3 = testPage2.getId(); - - pageDTO = applicationPageService.createPage(testPage2); - StepVerifier.create(pageDTO); - applicationPageService.publish(application.getId()); - - Mono applicationMono = applicationPageService.reorderPage(application.getId(), pageId1, 2); + // Create all the pages for this application in a blocking manner. + createApplicationMono + .flatMap(application -> { + PageDTO testPage = new PageDTO(); + testPage.setName("Page2"); + testPage.setApplicationId(application.getId()); + return applicationPageService.createPage(testPage) + .then(Mono.just(application)); + }) + .flatMap(application -> { + PageDTO testPage = new PageDTO(); + testPage.setName("Page3"); + testPage.setApplicationId(application.getId()); + return applicationPageService.createPage(testPage) + .then(Mono.just(application)); + }) + .flatMap(application -> { + PageDTO testPage = new PageDTO(); + testPage.setName("Page4"); + testPage.setApplicationId(application.getId()); + return applicationPageService.createPage(testPage); + }) + .block(); + String pageId = application.getPages().get(0).getId(); + Mono applicationM = createApplicationMono. + flatMap( application -> { + return applicationPageService.reorderPage(application.getId(), application.getPages().get(0).getId(), 3); + }); StepVerifier - .create(applicationMono) + .create(applicationM) .assertNext(application ->{ final List pages = application.getPages(); + assertThat(application.getPages().size()).isEqualTo(4); for(ApplicationPage page : pages){ - if(pageId3.equals(page.getId())){ - assertThat(page.getOrder()).isEqualTo(1); - } - if(pageId2.equals(page.getId())){ - assertThat(page.getOrder()).isEqualTo(0); - } - if(pageId1.equals(page.getId())){ - assertThat(page.getOrder()).isEqualTo(2); + if(pageId.equals(page.getId())){ + assertThat(page.getOrder()).isEqualTo(3); } } } ) From 1b36f79a4ec7d4537fe8577348ef971bf324c773 Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Wed, 16 Jun 2021 13:44:46 +0530 Subject: [PATCH 11/18] Fix merge conficts --- .../java/com/appsmith/server/migrations/DatabaseChangelog.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java index bc55f6a8b4..c2e965be5c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java @@ -2368,7 +2368,7 @@ public class DatabaseChangelog { * created pages of all the application present. * - [] */ - @ChangeSet(order = "071", id = "add-and-update-order-for-all-pages", author = "") + @ChangeSet(order = "072", id = "add-and-update-order-for-all-pages", author = "") public void addOrderToAllPagesOfApplication(MongoTemplate mongoTemplate) { for (Application application : mongoTemplate.findAll(Application.class)) { if(application.getPages() != null) { From 2ccc1ef1f2031d5f22ef8473c433b983a8989e01 Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Wed, 16 Jun 2021 14:29:03 +0530 Subject: [PATCH 12/18] Change the test scenario --- .../com/appsmith/server/services/PageServiceTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java index 4b23a1e6a3..dc075968e6 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java @@ -386,7 +386,7 @@ public class PageServiceTest { public void reOrderPageMovingUp() { Application app = new Application(); - app.setName("validGetApplicationPagesMultiPageApp-Test"); + app.setName("reOrderPageMovingUp-Test"); setupTestApplication(); Mono createApplicationMono = applicationPageService.createApplication(app, orgId) @@ -415,10 +415,10 @@ public class PageServiceTest { return applicationPageService.createPage(testPage); }) .block(); - String pageId = application.getPages().get(3).getId(); + String pageId = application.getPages().get(0).getId(); Mono applicationM = createApplicationMono. flatMap( application -> { - return applicationPageService.reorderPage(application.getId(), application.getPages().get(3).getId(), 0); + return applicationPageService.reorderPage(application.getId(), application.getPages().get(0).getId(), 0); }); StepVerifier .create(applicationM) @@ -439,7 +439,7 @@ public class PageServiceTest { public void reOrderPageMovingDown() { Application app = new Application(); - app.setName("validGetApplicationPagesMultiPageApp-Test"); + app.setName("reOrderPageMovingDown-Test"); setupTestApplication(); Mono createApplicationMono = applicationPageService.createApplication(app, orgId) From e119bd716c0582b6b64a6fed91b9814b3a9080cd Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Wed, 16 Jun 2021 21:12:18 +0530 Subject: [PATCH 13/18] Ignore the order of the page while checking the published and edited pages test --- .../services/ApplicationServiceTest.java | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java index 4bcf8ebe5c..607f5979f1 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java @@ -766,6 +766,7 @@ public class ApplicationServiceTest { ApplicationPage applicationPage = new ApplicationPage(); applicationPage.setId(newPage.getId()); applicationPage.setIsDefault(false); + applicationPage.setOrder(FieldName.DEFAULT_PAGE_ORDER+1); StepVerifier .create(applicationService.findById(newPage.getApplicationId(), MANAGE_APPLICATIONS)) @@ -811,25 +812,37 @@ public class ApplicationServiceTest { Mono updatedDefaultPageApplicationMono = applicationMono .flatMap(application -> applicationPageService.makePageDefault(application.getId(), newPage.getId())); - ApplicationPage unpublishedEditedPage = new ApplicationPage(); - unpublishedEditedPage.setId(newPage.getId()); - unpublishedEditedPage.setIsDefault(true); - ApplicationPage publishedEditedPage = new ApplicationPage(); publishedEditedPage.setId(newPage.getId()); publishedEditedPage.setIsDefault(false); + ApplicationPage unpublishedEditedPage = new ApplicationPage(); + unpublishedEditedPage.setId(newPage.getId()); + unpublishedEditedPage.setIsDefault(true); + StepVerifier .create(updatedDefaultPageApplicationMono) .assertNext(editedApplication -> { List publishedPages = editedApplication.getPublishedPages(); assertThat(publishedPages).size().isEqualTo(2); - assertThat(publishedPages).containsAnyOf(publishedEditedPage); + boolean isFound = false; + for( ApplicationPage page: publishedPages) { + if(page.getId().equals(publishedEditedPage.getId()) && page.getIsDefault() == publishedEditedPage.getIsDefault()) { + isFound = true; + } + } + assertThat(isFound).isTrue(); List editedApplicationPages = editedApplication.getPages(); assertThat(editedApplicationPages.size()).isEqualTo(2); - assertThat(editedApplicationPages).containsAnyOf(unpublishedEditedPage); + isFound = false; + for( ApplicationPage page: editedApplicationPages) { + if(page.getId().equals(unpublishedEditedPage.getId()) && page.getIsDefault() == unpublishedEditedPage.getIsDefault()) { + isFound = true; + } + } + assertThat(isFound).isTrue(); }) .verifyComplete(); } @@ -873,7 +886,13 @@ public class ApplicationServiceTest { .assertNext(viewApplication -> { List editedApplicationPages = viewApplication.getPages(); assertThat(editedApplicationPages.size()).isEqualTo(2); - assertThat(editedApplicationPages).containsAnyOf(applicationPage); + boolean isFound = false; + for( ApplicationPage page: editedApplicationPages) { + if(page.getId().equals(applicationPage.getId()) && page.getIsDefault() == applicationPage.getIsDefault()) { + isFound = true; + } + } + assertThat(isFound).isTrue(); }) .verifyComplete(); } From bd3369dd034c185a7b091cc1e6fa4de4ff6fbd41 Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Fri, 18 Jun 2021 10:25:36 +0530 Subject: [PATCH 14/18] Remove * import statements --- .../server/migrations/DatabaseChangelog.java | 30 ++++++++++++++++++- .../server/services/PageServiceTest.java | 17 ++++++----- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java index eb629728d3..e30975edac 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java @@ -14,7 +14,35 @@ import com.appsmith.server.acl.AclPermission; import com.appsmith.server.acl.AppsmithRole; import com.appsmith.server.constants.Appsmith; import com.appsmith.server.constants.FieldName; -import com.appsmith.server.domains.*; +import com.appsmith.server.domains.Action; +import com.appsmith.server.domains.Application; +import com.appsmith.server.domains.Collection; +import com.appsmith.server.domains.Config; +import com.appsmith.server.domains.Datasource; +import com.appsmith.server.domains.Group; +import com.appsmith.server.domains.InviteUser; +import com.appsmith.server.domains.Layout; +import com.appsmith.server.domains.NewAction; +import com.appsmith.server.domains.NewPage; +import com.appsmith.server.domains.Organization; +import com.appsmith.server.domains.OrganizationPlugin; +import com.appsmith.server.domains.Page; +import com.appsmith.server.domains.PasswordResetToken; +import com.appsmith.server.domains.Permission; +import com.appsmith.server.domains.Plugin; +import com.appsmith.server.domains.PluginType; +import com.appsmith.server.domains.QApplication; +import com.appsmith.server.domains.QConfig; +import com.appsmith.server.domains.QDatasource; +import com.appsmith.server.domains.QNewAction; +import com.appsmith.server.domains.QOrganization; +import com.appsmith.server.domains.QPlugin; +import com.appsmith.server.domains.Role; +import com.appsmith.server.domains.Sequence; +import com.appsmith.server.domains.User; +import com.appsmith.server.domains.UserData; +import com.appsmith.server.domains.UserRole; +import com.appsmith.server.domains.ApplicationPage; import com.appsmith.server.dtos.ActionDTO; import com.appsmith.server.dtos.DslActionDTO; import com.appsmith.server.dtos.OrganizationPluginStatus; diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java index dc075968e6..69b033260d 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java @@ -4,7 +4,13 @@ import com.appsmith.external.models.ActionConfiguration; import com.appsmith.external.models.Policy; import com.appsmith.external.plugins.PluginExecutor; import com.appsmith.server.constants.FieldName; -import com.appsmith.server.domains.*; +import com.appsmith.server.domains.Application; +import com.appsmith.server.domains.Datasource; +import com.appsmith.server.domains.Layout; +import com.appsmith.server.domains.NewAction; +import com.appsmith.server.domains.Plugin; +import com.appsmith.server.domains.User; +import com.appsmith.server.domains.ApplicationPage; import com.appsmith.server.dtos.ActionDTO; import com.appsmith.server.dtos.LayoutDTO; import com.appsmith.server.dtos.PageDTO; @@ -451,23 +457,20 @@ public class PageServiceTest { PageDTO testPage = new PageDTO(); testPage.setName("Page2"); testPage.setApplicationId(application.getId()); - return applicationPageService.createPage(testPage) - .then(Mono.just(application)); + return applicationPageService.createPage(testPage); }) .flatMap(application -> { PageDTO testPage = new PageDTO(); testPage.setName("Page3"); testPage.setApplicationId(application.getId()); - return applicationPageService.createPage(testPage) - .then(Mono.just(application)); + return applicationPageService.createPage(testPage); }) .flatMap(application -> { PageDTO testPage = new PageDTO(); testPage.setName("Page4"); testPage.setApplicationId(application.getId()); return applicationPageService.createPage(testPage); - }) - .block(); + }); String pageId = application.getPages().get(0).getId(); Mono applicationM = createApplicationMono. flatMap( application -> { From e02e572df7b1465c5e092e9c6c6c787931a8f4b1 Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Fri, 18 Jun 2021 14:53:33 +0530 Subject: [PATCH 15/18] Change the logic to have check for the order of all the pages --- .../services/ApplicationPageServiceImpl.java | 4 +- .../server/services/PageServiceTest.java | 133 ++++++++++-------- 2 files changed, 78 insertions(+), 59 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java index c66dcc7409..463ac7a976 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java @@ -696,7 +696,7 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { * Case 2(isMovingUp == false): p2 to p5, order of p3,p4,p5 decreases by 1. **/ if(foundPage != null){ - boolean isMovingUp = order > foundPage.getOrder(); + boolean isMovingUp = order < foundPage.getOrder(); if(isMovingUp){ for (final ApplicationPage page : pages) { if (page.getOrder() < foundPage.getOrder() && page.getOrder() >= order) { @@ -705,7 +705,7 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { } } else{ for (final ApplicationPage page : pages) { - if (page.getOrder() >= foundPage.getOrder() && page.getOrder() < order) { + if (page.getOrder() > foundPage.getOrder() && page.getOrder() <= order) { page.setOrder(page.getOrder()-1); } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java index 69b033260d..3c2946261a 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java @@ -391,50 +391,58 @@ public class PageServiceTest { @WithUserDetails(value = "api_user") public void reOrderPageMovingUp() { - Application app = new Application(); - app.setName("reOrderPageMovingUp-Test"); - setupTestApplication(); + User apiUser = userService.findByEmail("api_user").block(); + orgId = apiUser.getOrganizationIds().iterator().next(); + Application newApp = new Application(); + newApp.setName(UUID.randomUUID().toString()); - Mono createApplicationMono = applicationPageService.createApplication(app, orgId) - .cache(); + application = applicationPageService.createApplication(newApp, orgId).block(); + applicationId = application.getId(); + final String[] pageIds = new String[4]; - // Create all the pages for this application in a blocking manner. - createApplicationMono - .flatMap(application -> { - PageDTO testPage = new PageDTO(); - testPage.setName("Page2"); - testPage.setApplicationId(application.getId()); - return applicationPageService.createPage(testPage) - .then(Mono.just(application)); - }) - .flatMap(application -> { + PageDTO testPage1 = new PageDTO(); + testPage1.setName("Page2"); + testPage1.setApplicationId(applicationId); + Mono applicationPageReOrdered = applicationPageService.createPage(testPage1) + .flatMap(pageDTO -> { PageDTO testPage = new PageDTO(); testPage.setName("Page3"); - testPage.setApplicationId(application.getId()); - return applicationPageService.createPage(testPage) - .then(Mono.just(application)); - }) - .flatMap(application -> { - PageDTO testPage = new PageDTO(); - testPage.setName("Page4"); - testPage.setApplicationId(application.getId()); + testPage.setApplicationId(applicationId); return applicationPageService.createPage(testPage); }) - .block(); - String pageId = application.getPages().get(0).getId(); - Mono applicationM = createApplicationMono. - flatMap( application -> { - return applicationPageService.reorderPage(application.getId(), application.getPages().get(0).getId(), 0); + .flatMap(pageDTO -> { + PageDTO testPage = new PageDTO(); + testPage.setName("Page4"); + testPage.setApplicationId(applicationId); + return applicationPageService.createPage(testPage); + }) + .flatMap(pageDTO -> applicationService.getById(pageDTO.getApplicationId())) + .flatMap( application -> { + pageIds[0] = application.getPages().get(0).getId(); + pageIds[1] = application.getPages().get(1).getId(); + pageIds[2] = application.getPages().get(2).getId(); + pageIds[3] = application.getPages().get(3).getId(); + return applicationPageService.reorderPage(application.getId(), application.getPages().get(3).getId(), 1); }); + StepVerifier - .create(applicationM) - .assertNext(application ->{ + .create(applicationPageReOrdered) + .assertNext(application -> { final List pages = application.getPages(); assertThat(application.getPages().size()).isEqualTo(4); - for(ApplicationPage page : pages){ - if(pageId.equals(page.getId())){ + for(ApplicationPage page : pages) { + if(pageIds[0].equals(page.getId())) { assertThat(page.getOrder()).isEqualTo(0); } + if(pageIds[1].equals(page.getId())) { + assertThat(page.getOrder()).isEqualTo(2); + } + if(pageIds[2].equals(page.getId())) { + assertThat(page.getOrder()).isEqualTo(3); + } + if(pageIds[3].equals(page.getId())) { + assertThat(page.getOrder()).isEqualTo(1); + } } } ) .verifyComplete(); @@ -444,47 +452,58 @@ public class PageServiceTest { @WithUserDetails(value ="api_user") public void reOrderPageMovingDown() { - Application app = new Application(); - app.setName("reOrderPageMovingDown-Test"); - setupTestApplication(); + User apiUser = userService.findByEmail("api_user").block(); + orgId = apiUser.getOrganizationIds().iterator().next(); + Application newApp = new Application(); + newApp.setName(UUID.randomUUID().toString()); - Mono createApplicationMono = applicationPageService.createApplication(app, orgId) - .cache(); + application = applicationPageService.createApplication(newApp, orgId).block(); + applicationId = application.getId(); + final String[] pageIds = new String[4]; - // Create all the pages for this application in a blocking manner. - createApplicationMono - .flatMap(application -> { - PageDTO testPage = new PageDTO(); - testPage.setName("Page2"); - testPage.setApplicationId(application.getId()); - return applicationPageService.createPage(testPage); - }) - .flatMap(application -> { + PageDTO testPage1 = new PageDTO(); + testPage1.setName("Page2"); + testPage1.setApplicationId(applicationId); + Mono applicationPageReOrdered = applicationPageService.createPage(testPage1) + .flatMap(pageDTO -> { PageDTO testPage = new PageDTO(); testPage.setName("Page3"); - testPage.setApplicationId(application.getId()); + testPage.setApplicationId(applicationId); return applicationPageService.createPage(testPage); }) - .flatMap(application -> { + .flatMap(pageDTO -> { PageDTO testPage = new PageDTO(); testPage.setName("Page4"); - testPage.setApplicationId(application.getId()); + testPage.setApplicationId(applicationId); return applicationPageService.createPage(testPage); - }); - String pageId = application.getPages().get(0).getId(); - Mono applicationM = createApplicationMono. - flatMap( application -> { + }) + .flatMap(pageDTO -> applicationService.getById(pageDTO.getApplicationId())) + .flatMap( application -> { + pageIds[0] = application.getPages().get(0).getId(); + pageIds[1] = application.getPages().get(1).getId(); + pageIds[2] = application.getPages().get(2).getId(); + pageIds[3] = application.getPages().get(3).getId(); return applicationPageService.reorderPage(application.getId(), application.getPages().get(0).getId(), 3); }); + StepVerifier - .create(applicationM) - .assertNext(application ->{ + .create(applicationPageReOrdered) + .assertNext(application -> { final List pages = application.getPages(); assertThat(application.getPages().size()).isEqualTo(4); - for(ApplicationPage page : pages){ - if(pageId.equals(page.getId())){ + for(ApplicationPage page : pages) { + if(pageIds[0].equals(page.getId())) { assertThat(page.getOrder()).isEqualTo(3); } + if(pageIds[1].equals(page.getId())) { + assertThat(page.getOrder()).isEqualTo(0); + } + if(pageIds[2].equals(page.getId())) { + assertThat(page.getOrder()).isEqualTo(1); + } + if(pageIds[3].equals(page.getId())) { + assertThat(page.getOrder()).isEqualTo(2); + } } } ) .verifyComplete(); From 9ad742e97e6b616d6f574655ff786396d31bc52e Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Fri, 18 Jun 2021 22:40:16 +0530 Subject: [PATCH 16/18] Change the test method name to more meaningful,.equals for boolean comparision, fix indentation --- .../java/com/appsmith/server/constants/FieldName.java | 1 - .../server/services/ApplicationPageServiceImpl.java | 8 ++++---- .../appsmith/server/services/ApplicationServiceTest.java | 8 ++++---- .../com/appsmith/server/services/PageServiceTest.java | 4 ++-- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/FieldName.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/FieldName.java index 51ad052619..3b044a72c4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/FieldName.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/constants/FieldName.java @@ -71,5 +71,4 @@ public class FieldName { public static String MONGO_ESCAPE_CLASS = "appsmith_mongo_escape_class"; public static String MONGO_UNESCAPED_ID = "_id"; public static String MONGO_UNESCAPED_CLASS = "_class"; - public static Integer DEFAULT_PAGE_ORDER = 0; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java index 463ac7a976..c50ff1b9a4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java @@ -139,7 +139,7 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { */ @Override public Mono addPageToApplication(Application application, PageDTO page, Boolean isDefault) { - Integer order = application.getPages() != null ? application.getPages().size() : FieldName.DEFAULT_PAGE_ORDER; + Integer order = application.getPages() != null ? application.getPages().size() : 0; return applicationRepository.addPageToApplication(application.getId(), page.getId(), isDefault, order) .doOnSuccess(result -> { if (result.getModifiedCount() != 1) { @@ -695,15 +695,15 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { * * Case 2(isMovingUp == false): p2 to p5, order of p3,p4,p5 decreases by 1. **/ - if(foundPage != null){ + if(foundPage != null) { boolean isMovingUp = order < foundPage.getOrder(); - if(isMovingUp){ + if(isMovingUp) { for (final ApplicationPage page : pages) { if (page.getOrder() < foundPage.getOrder() && page.getOrder() >= order) { page.setOrder(page.getOrder()+1); } } - } else{ + } else { for (final ApplicationPage page : pages) { if (page.getOrder() > foundPage.getOrder() && page.getOrder() <= order) { page.setOrder(page.getOrder()-1); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java index 607f5979f1..12986ed1fc 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java @@ -766,7 +766,7 @@ public class ApplicationServiceTest { ApplicationPage applicationPage = new ApplicationPage(); applicationPage.setId(newPage.getId()); applicationPage.setIsDefault(false); - applicationPage.setOrder(FieldName.DEFAULT_PAGE_ORDER+1); + applicationPage.setOrder(1); StepVerifier .create(applicationService.findById(newPage.getApplicationId(), MANAGE_APPLICATIONS)) @@ -828,7 +828,7 @@ public class ApplicationServiceTest { assertThat(publishedPages).size().isEqualTo(2); boolean isFound = false; for( ApplicationPage page: publishedPages) { - if(page.getId().equals(publishedEditedPage.getId()) && page.getIsDefault() == publishedEditedPage.getIsDefault()) { + if(page.getId().equals(publishedEditedPage.getId()) && page.getIsDefault().equals(publishedEditedPage.getIsDefault())) { isFound = true; } } @@ -838,7 +838,7 @@ public class ApplicationServiceTest { assertThat(editedApplicationPages.size()).isEqualTo(2); isFound = false; for( ApplicationPage page: editedApplicationPages) { - if(page.getId().equals(unpublishedEditedPage.getId()) && page.getIsDefault() == unpublishedEditedPage.getIsDefault()) { + if(page.getId().equals(unpublishedEditedPage.getId()) && page.getIsDefault().equals(unpublishedEditedPage.getIsDefault())) { isFound = true; } } @@ -888,7 +888,7 @@ public class ApplicationServiceTest { assertThat(editedApplicationPages.size()).isEqualTo(2); boolean isFound = false; for( ApplicationPage page: editedApplicationPages) { - if(page.getId().equals(applicationPage.getId()) && page.getIsDefault() == applicationPage.getIsDefault()) { + if(page.getId().equals(applicationPage.getId()) && page.getIsDefault().equals(applicationPage.getIsDefault())) { isFound = true; } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java index 3c2946261a..2560f4c393 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java @@ -389,7 +389,7 @@ public class PageServiceTest { @Test @WithUserDetails(value = "api_user") - public void reOrderPageMovingUp() { + public void reOrderPageFromHighOrderToLowOrder() { User apiUser = userService.findByEmail("api_user").block(); orgId = apiUser.getOrganizationIds().iterator().next(); @@ -450,7 +450,7 @@ public class PageServiceTest { @Test @WithUserDetails(value ="api_user") - public void reOrderPageMovingDown() { + public void reOrderPageFromLowOrderToHighOrder() { User apiUser = userService.findByEmail("api_user").block(); orgId = apiUser.getOrganizationIds().iterator().next(); From fa9a1bc48858924ed1e6433d089160ff45244b8d Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Wed, 23 Jun 2021 10:51:25 +0530 Subject: [PATCH 17/18] Set the order to published pages --- .../server/migrations/DatabaseChangelog.java | 60 +++++++++---------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java index 859afb2a82..39db32ec1d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java @@ -2457,37 +2457,6 @@ public class DatabaseChangelog { } } } - } - - /** - * - Older order file where not present for the pages created within the application because page reordering with in - * the application was not supported. - * - New Form order field will be added to the Page object and is used to order the pages with in the application - * Since the previously created pages doesnt have the order, we will be updating/adding order to all the previously - * created pages of all the application present. - * - [] - */ - @ChangeSet(order = "072", id = "add-and-update-order-for-all-pages", author = "") - public void addOrderToAllPagesOfApplication(MongoTemplate mongoTemplate) { - for (Application application : mongoTemplate.findAll(Application.class)) { - if(application.getPages() != null) { - int i = 0; - for (ApplicationPage page : application.getPages()) { - page.setOrder(i); - i++; - } - if(application.getPublishedPages() != null) { - for (ApplicationPage page : application.getPublishedPages()) { - for(ApplicationPage npage: application.getPages()) { - if(npage.getId().equals(page.getId())) { - page.setOrder(npage.getOrder()); - } - } - } - } - mongoTemplate.save(application); - } - } } private List generateMongoFormConfigTemplates(Map configuration) { @@ -2524,6 +2493,35 @@ public class DatabaseChangelog { } + /** + * - Older order file where not present for the pages created within the application because page reordering with in + * the application was not supported. + * - New Form order field will be added to the Page object and is used to order the pages with in the application + * Since the previously created pages doesnt have the order, we will be updating/adding order to all the previously + * created pages of all the application present. + * - [] + */ + @ChangeSet(order = "073", id = "add-and-update-order-for-all-pages", author = "") + public void addOrderToAllPagesOfApplication(MongoTemplate mongoTemplate) { + for (Application application : mongoTemplate.findAll(Application.class)) { + if(application.getPages() != null) { + int i = 0; + for (ApplicationPage page : application.getPages()) { + page.setOrder(i); + i++; + } + if(application.getPublishedPages() != null) { + i = 0; + for (ApplicationPage page : application.getPublishedPages()) { + page.setOrder(i); + i++; + } + } + mongoTemplate.save(application); + } + } + } + // @ChangeSet(order = "073", id = "mongo-form-merge-update-commands", author = "") // public void migrateUpdateOneToUpdateManyMongoFormCommand(MongockTemplate mongockTemplate) { // From a1defb954400030abe1bb702bb031cd3def408a2 Mon Sep 17 00:00:00 2001 From: Anagh Hegde Date: Wed, 23 Jun 2021 11:16:15 +0530 Subject: [PATCH 18/18] Remove the unnecessary comments --- .../server/migrations/DatabaseChangelog.java | 71 +------------------ 1 file changed, 2 insertions(+), 69 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java index 5e4c840db2..55207d1830 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java @@ -2455,6 +2455,8 @@ public class DatabaseChangelog { .users(adminUsernames).build(); application.getPolicies().add(newExportAppPolicy); } + + mongoTemplate.save(application); } } } @@ -2493,75 +2495,6 @@ public class DatabaseChangelog { } - -// @ChangeSet(order = "073", id = "mongo-form-merge-update-commands", author = "") -// public void migrateUpdateOneToUpdateManyMongoFormCommand(MongockTemplate mongockTemplate) { -// -// Plugin mongoPlugin = mongockTemplate.findOne(query(where("packageName").is("mongo-plugin")), Plugin.class); -// -// // Fetch all the actions built on top of a mongo database with command type update_one or update_many -// assert mongoPlugin != null; -// List updateMongoActions = mongockTemplate.find( -// query(new Criteria().andOperator( -// where(fieldName(QNewAction.newAction.pluginId)).is(mongoPlugin.getId()))), -// NewAction.class -// ) -// .stream() -// .filter(mongoAction -> { -// if (mongoAction.getUnpublishedAction() == null || mongoAction.getUnpublishedAction().getActionConfiguration() == null) { -// return false; -// } -// final List pluginSpecifiedTemplates = mongoAction.getUnpublishedAction().getActionConfiguration().getPluginSpecifiedTemplates(); -// -// // Filter out all the actions which are of either of the two update command type -// if (pluginSpecifiedTemplates != null && pluginSpecifiedTemplates.size() == 21) { -// Property commandProperty = pluginSpecifiedTemplates.get(2); -// if (commandProperty != null && commandProperty.getValue() != null) { -// String command = (String) commandProperty.getValue(); -// if (command.equals("UPDATE_ONE") || command.equals("UPDATE_MANY")) { -// return true; -// } -// } -// } -// // Not an action of interest for migration. -// return false; -// }) -// .collect(Collectors.toList()); -// -// for (NewAction action : updateMongoActions) { -// List pluginSpecifiedTemplates = action.getUnpublishedAction().getActionConfiguration().getPluginSpecifiedTemplates(); -// String command = (String) pluginSpecifiedTemplates.get(2).getValue(); -// -// // Set the command name to be UPDATE for both the update commands -// pluginSpecifiedTemplates.get(2).setValue("UPDATE"); -// -// // In case of update one, migrate the query and update configurations. -// if (command.equals("UPDATE_ONE")) { -// String query = (String) pluginSpecifiedTemplates.get(8).getValue(); -// String update = (String) pluginSpecifiedTemplates.get(10).getValue(); -// -// Map configMap = new HashMap<>(); -// configMap.put(0, pluginSpecifiedTemplates.get(0).getValue()); -// configMap.put(1, "FORM"); -// configMap.put(2, "UPDATE"); -// configMap.put(19, pluginSpecifiedTemplates.get(19).getValue()); -// // Query for all the documents in the collection -// configMap.put(11, query); -// configMap.put(12, update); -// configMap.put(21, "SINGLE"); -// -// List updatedTemplates = generateMongoFormConfigTemplates(configMap); -// action.getUnpublishedAction().getActionConfiguration().setPluginSpecifiedTemplates(updatedTemplates); -// -// } -// } -// -// // Now that all the actions have been updated, save all the actions -// for (NewAction action : updateMongoActions) { -// mongockTemplate.save(action); -// } -// } - @ChangeSet(order = "073", id = "mongo-form-merge-update-commands", author = "") public void migrateUpdateOneToUpdateManyMongoFormCommand(MongockTemplate mongockTemplate) {