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 4e3d1269be..c65bdaefe4 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 @@ -4,6 +4,7 @@ import com.appsmith.server.constants.Url; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.ApplicationJson; import com.appsmith.server.dtos.ApplicationAccessDTO; +import com.appsmith.server.dtos.ApplicationPagesDTO; import com.appsmith.server.dtos.ResponseDTO; import com.appsmith.server.dtos.UserHomepageDTO; import com.appsmith.server.exceptions.AppsmithError; @@ -94,7 +95,7 @@ public class ApplicationController extends BaseController> reorderPage( + public Mono> reorderPage( @PathVariable String applicationId, @PathVariable String pageId, @RequestParam Integer order 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..b304224291 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 @@ -20,8 +20,6 @@ public class ApplicationPage { Boolean isDefault; - Integer order; - @JsonIgnore public boolean isDefault() { return Boolean.TRUE.equals(isDefault); 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 931fb049c6..19d2e91f22 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,7 +2455,7 @@ public class DatabaseChangelog { .users(adminUsernames).build(); application.getPolicies().add(newExportAppPolicy); } - + mongoTemplate.save(application); } } @@ -2578,12 +2578,12 @@ public class DatabaseChangelog { } } - + @ChangeSet(order = "074", id = "ensure-user-created-and-updated-at-fields", author = "") public void ensureUserCreatedAndUpdatedAt(MongoTemplate mongoTemplate) { final List missingCreatedAt = mongoTemplate.find( - query(where("createdAt").exists(false)), - User.class + query(where("createdAt").exists(false)), + User.class ); for (User user : missingCreatedAt) { @@ -2592,8 +2592,8 @@ public class DatabaseChangelog { } final List missingUpdatedAt = mongoTemplate.find( - query(where("updatedAt").exists(false)), - User.class + query(where("updatedAt").exists(false)), + User.class ); for (User user : missingUpdatedAt) { @@ -2601,7 +2601,7 @@ public class DatabaseChangelog { mongoTemplate.save(user); } } - + /** * - Older order file where not present for the pages created within the application because page reordering with in * the application was not supported. @@ -2613,7 +2613,8 @@ public class DatabaseChangelog { @ChangeSet(order = "075", 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) { + //Commenting out this piece code as we have decided to remove the order field from ApplicationPages + /*if(application.getPages() != null) { int i = 0; for (ApplicationPage page : application.getPages()) { page.setOrder(i); @@ -2627,7 +2628,7 @@ public class DatabaseChangelog { } } mongoTemplate.save(application); - } + }*/ } } @@ -2799,4 +2800,17 @@ public class DatabaseChangelog { mongoTemplate.save(plugin); } } + + @ChangeSet(order = "079", id = "remove-order-field-from-application- pages", author = "" ) + public void removePageOrderFieldFromApplicationPages(MongockTemplate mongoTemplate) { + Query query = new Query(); + query.addCriteria(Criteria.where("pages").exists(TRUE)); + + Update update = new Update(); + update.unset("pages.$[].order"); + mongoTemplate.updateMulti(query(where("pages").exists(TRUE)), update, Application.class); + + update.unset("publishedPages.$[].order"); + mongoTemplate.updateMulti(query(where("publishedPages").exists(TRUE)), update, Application.class); + } } 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 80d1aa0761..afe5bdbe66 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, Integer order); + Mono addPageToApplication(String applicationId, String pageId, boolean isDefault); 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 93ed1e9f84..f3b25f2223 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,11 +73,11 @@ public class CustomApplicationRepositoryImpl extends BaseAppsmithRepositoryImpl< } @Override - public Mono addPageToApplication(String applicationId, String pageId, boolean isDefault, Integer order) { - final ApplicationPage applicationPage = new ApplicationPage(pageId, isDefault, order); + public Mono addPageToApplication(String applicationId, String pageId, boolean isDefault) { + final ApplicationPage applicationPage = new ApplicationPage(pageId, isDefault); return mongoOperations.updateFirst( Query.query(getIdCriteria(applicationId)), - new Update().addToSet(fieldName(QApplication.application.pages), applicationPage), + new Update().push(fieldName(QApplication.application.pages), applicationPage), 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 09ca039955..5bf9371e8c 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 @@ -2,6 +2,7 @@ package com.appsmith.server.services; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.User; +import com.appsmith.server.dtos.ApplicationPagesDTO; import com.appsmith.server.dtos.PageDTO; import com.mongodb.client.result.UpdateResult; import reactor.core.publisher.Mono; @@ -39,5 +40,5 @@ public interface ApplicationPageService { Mono sendApplicationPublishedEvent(Application application); - Mono reorderPage(String applicationId, String pageId, Integer order); + 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 c3c8c5eb41..ea712d996a 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,13 +139,28 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { */ @Override public Mono addPageToApplication(Application application, PageDTO page, Boolean isDefault) { - Integer order = application.getPages() != null ? application.getPages().size() : 0; - 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."); - } - }); + if(isDuplicatePage(application, page.getId())) { + return applicationRepository.addPageToApplication(application.getId(), page.getId(), isDefault) + .doOnSuccess(result -> { + if (result.getModifiedCount() != 1) { + log.error("Add page to application didn't update anything, probably because application wasn't found."); + } + }); + } else{ + return Mono.error(new AppsmithException(AppsmithError.DUPLICATE_KEY, "Page already exists with id "+page.getId())); + } + + } + + private Boolean isDuplicatePage(Application application, String pageId) { + if( application.getPages() != null) { + int count = (int) application.getPages().stream().filter( + applicationPage -> applicationPage.getId().equals(pageId)).count(); + if (count > 0) { + return Boolean.FALSE; + } + } + return Boolean.TRUE; } @Override @@ -676,12 +691,12 @@ public class ApplicationPageServiceImpl implements ApplicationPageService { * @return Application object with the latest order **/ @Override - public Mono reorderPage(String applicationId, String pageId, Integer order) { + 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(); + List pages = application.getPages(); ApplicationPage foundPage = null; for (final ApplicationPage page : pages) { @@ -690,32 +705,14 @@ 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) { - 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 the given order - foundPage.setOrder(order); + pages.remove(foundPage); + pages.add(order, foundPage); } + return applicationRepository .setPages(applicationId, pages) - .then(applicationService.getById(applicationId)); + .then(newPageService.findApplicationPagesByApplicationIdAndViewMode(applicationId,Boolean.FALSE)); }); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewPageServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewPageServiceImpl.java index 6bd11ee721..6a2922b85f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewPageServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewPageServiceImpl.java @@ -4,6 +4,7 @@ import com.appsmith.server.acl.AclPermission; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.Application; import com.appsmith.server.domains.ApplicationPage; +import com.appsmith.server.domains.Collection; import com.appsmith.server.domains.Layout; import com.appsmith.server.domains.NewPage; import com.appsmith.server.dtos.ApplicationPagesDTO; @@ -27,7 +28,11 @@ import reactor.core.scheduler.Scheduler; import javax.validation.Validator; import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -210,12 +215,31 @@ public class NewPageServiceImpl extends BaseService repository.findAllByIds(pageIds, READ_PAGES)) .collectList() - .zipWith(defaultPageIdMono) - .flatMap(tuple -> { + .flatMap( pagesFromDb -> Mono.zip( + Mono.just(pagesFromDb), + defaultPageIdMono, + applicationMono + )).flatMap(tuple -> { List pagesFromDb = tuple.getT1(); String defaultPageId = tuple.getT2(); List pageNameIdDTOList = new ArrayList<>(); + List pages = tuple.getT3().getPages(); + List publishedPages = tuple.getT3().getPublishedPages(); + Map pagesOrder = new HashMap<>(); + Map publishedPagesOrder = new HashMap<>(); + + if(Boolean.TRUE.equals(view)) { + for (int i = 0; i < publishedPages.size(); i++) + { + publishedPagesOrder.put(publishedPages.get(i).getId(), i); + } + } else { + for (int i = 0; i < pages.size(); i++) + { + pagesOrder.put(pages.get(i).getId(), i); + } + } for (NewPage pageFromDb : pagesFromDb) { @@ -241,10 +265,15 @@ public class NewPageServiceImpl extends BaseService publishedPages.indexOf(item))); + } else { + Collections.sort(pageNameIdDTOList, + Comparator.comparing(item -> pages.indexOf(item))); + } return Mono.just(pageNameIdDTOList); }); 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 a2c7e31482..134f6d3b65 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 @@ -793,7 +793,6 @@ public class ApplicationServiceTest { ApplicationPage applicationPage = new ApplicationPage(); applicationPage.setId(newPage.getId()); applicationPage.setIsDefault(false); - applicationPage.setOrder(1); StepVerifier .create(applicationService.findById(newPage.getApplicationId(), MANAGE_APPLICATIONS)) 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 2560f4c393..6f82c1004f 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 @@ -12,8 +12,10 @@ 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.ApplicationPagesDTO; import com.appsmith.server.dtos.LayoutDTO; import com.appsmith.server.dtos.PageDTO; +import com.appsmith.server.dtos.PageNameIdDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.MockPluginExecutor; @@ -403,7 +405,7 @@ public class PageServiceTest { PageDTO testPage1 = new PageDTO(); testPage1.setName("Page2"); testPage1.setApplicationId(applicationId); - Mono applicationPageReOrdered = applicationPageService.createPage(testPage1) + Mono applicationPageReOrdered = applicationPageService.createPage(testPage1) .flatMap(pageDTO -> { PageDTO testPage = new PageDTO(); testPage.setName("Page3"); @@ -428,22 +430,12 @@ public class PageServiceTest { StepVerifier .create(applicationPageReOrdered) .assertNext(application -> { - final List pages = application.getPages(); + final List pages = application.getPages(); assertThat(application.getPages().size()).isEqualTo(4); - 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); - } - } + assertThat(application.getPages().get(0).getId().equals(pageIds[0])); + assertThat(application.getPages().get(1).getId().equals(pageIds[3])); + assertThat(application.getPages().get(2).getId().equals(pageIds[1])); + assertThat(application.getPages().get(3).getId().equals(pageIds[2])); } ) .verifyComplete(); } @@ -464,7 +456,7 @@ public class PageServiceTest { PageDTO testPage1 = new PageDTO(); testPage1.setName("Page2"); testPage1.setApplicationId(applicationId); - Mono applicationPageReOrdered = applicationPageService.createPage(testPage1) + Mono applicationPageReOrdered = applicationPageService.createPage(testPage1) .flatMap(pageDTO -> { PageDTO testPage = new PageDTO(); testPage.setName("Page3"); @@ -489,26 +481,40 @@ public class PageServiceTest { StepVerifier .create(applicationPageReOrdered) .assertNext(application -> { - final List pages = application.getPages(); + final List pages = application.getPages(); assertThat(application.getPages().size()).isEqualTo(4); - 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); - } - } + assertThat(application.getPages().get(3).getId().equals(pageIds[0])); + assertThat(application.getPages().get(0).getId().equals(pageIds[1])); + assertThat(application.getPages().get(1).getId().equals(pageIds[2])); + assertThat(application.getPages().get(2).getId().equals(pageIds[3])); } ) .verifyComplete(); } + @Test + @WithUserDetails(value = "api_user") + public void addDuplicatePageToApplication() { + + PageDTO testPage = new PageDTO(); + testPage.setName("PageServiceTest TestApp"); + setupTestApplication(); + testPage.setApplicationId(application.getId()); + + Mono pageMono = applicationPageService.createPage(testPage) + .flatMap(pageDTO -> { + PageDTO testPage1 = new PageDTO(); + testPage1.setName("Page3"); + testPage1.setApplicationId(applicationId); + testPage1.setId(pageDTO.getId()); + return applicationPageService.createPage(testPage1); + }); + StepVerifier + .create(pageMono) + .expectErrorMatches(throwable -> throwable instanceof AppsmithException) + .verify(); + } + + @After public void purgeAllPages() { newPageService.deleteAll();