zhangmu
/
pageplug
0
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Feature/page order API refactor (#5799) 

* Refactor Page order API - Instead of the order filed use the Order persistency in Array offered by Mongo

* Add migration script to remove the order filed from the Application Pages

* Change unit tests as per the new implementation

* Fix indentation issues

* Compute pages based on the view mode

* Removed order field in response and changed the page order API response to List of pages instead of the application object

* Remove the Order filed from the Response class

* Use the $push instead of $addToSet to maintain order of pages in array
This commit is contained in:
Anagh Hegde 2021-07-19 15:12:29 +05:30 committed by GitHub
parent 9a4b9c27cd
commit d5d6c3fa9c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 130 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ApplicationController.java
View File

@ -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<ApplicationService, Ap
}
@PutMapping("/{applicationId}/page/{pageId}/reorder")
public Mono<ResponseDTO<Application>> reorderPage(
public Mono<ResponseDTO<ApplicationPagesDTO>> reorderPage(
@PathVariable String applicationId,
@PathVariable String pageId,
@RequestParam Integer order

2
app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ApplicationPage.java
View File

@ -20,8 +20,6 @@ public class ApplicationPage {
Boolean isDefault;
Integer order;
@JsonIgnore
public boolean isDefault() {
return Boolean.TRUE.equals(isDefault);

32
app/server/appsmith-server/src/main/java/com/appsmith/server/migrations/DatabaseChangelog.java
View File

@ -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<User> 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<User> 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);
}
}

2
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepository.java
View File

@ -22,7 +22,7 @@ public interface CustomApplicationRepository extends AppsmithRepository<Applicat
Flux<Application> findByClonedFromApplicationId(String applicationId, AclPermission permission);
Mono<UpdateResult> addPageToApplication(String applicationId, String pageId, boolean isDefault, Integer order);
Mono<UpdateResult> addPageToApplication(String applicationId, String pageId, boolean isDefault);
Mono<UpdateResult> setPages(String applicationId, List<ApplicationPage> pages);

6
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomApplicationRepositoryImpl.java
View File

@ -73,11 +73,11 @@ public class CustomApplicationRepositoryImpl extends BaseAppsmithRepositoryImpl<
}
@Override
public Mono<UpdateResult> addPageToApplication(String applicationId, String pageId, boolean isDefault, Integer order) {
final ApplicationPage applicationPage = new ApplicationPage(pageId, isDefault, order);
public Mono<UpdateResult> 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
);
}

3
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageService.java
View File

@ -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<Void> sendApplicationPublishedEvent(Application application);
Mono<Application> reorderPage(String applicationId, String pageId, Integer order);
Mono<ApplicationPagesDTO> reorderPage(String applicationId, String pageId, Integer order);
}

59
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationPageServiceImpl.java
View File

@ -139,13 +139,28 @@ public class ApplicationPageServiceImpl implements ApplicationPageService {
*/
@Override
public Mono<UpdateResult> 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<Application> reorderPage(String applicationId, String pageId, Integer order) {
public Mono<ApplicationPagesDTO> 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<ApplicationPage> pages = application.getPages();
List<ApplicationPage> 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));
});
}

37
app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewPageServiceImpl.java
View File

@ -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<NewPageRepository, NewPage,
})
.flatMapMany(pageIds -> repository.findAllByIds(pageIds, READ_PAGES))
.collectList()
.zipWith(defaultPageIdMono)
.flatMap(tuple -> {
.flatMap( pagesFromDb -> Mono.zip(
Mono.just(pagesFromDb),
defaultPageIdMono,
applicationMono
)).flatMap(tuple -> {
List<NewPage> pagesFromDb = tuple.getT1();
String defaultPageId = tuple.getT2();
List<PageNameIdDTO> pageNameIdDTOList = new ArrayList<>();
List<ApplicationPage> pages = tuple.getT3().getPages();
List<ApplicationPage> publishedPages = tuple.getT3().getPublishedPages();
Map<String, Integer> pagesOrder = new HashMap<>();
Map<String, Integer> 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<NewPageRepository, NewPage,
} else {
pageNameIdDTO.setIsDefault(false);
}
pageNameIdDTOList.add(pageNameIdDTO);
}
if(Boolean.TRUE.equals(view)) {
Collections.sort(pageNameIdDTOList,
Comparator.comparing(item -> publishedPages.indexOf(item)));
} else {
Collections.sort(pageNameIdDTOList,
Comparator.comparing(item -> pages.indexOf(item)));
}
return Mono.just(pageNameIdDTOList);
});

1
app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java
View File

@ -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))

70
app/server/appsmith-server/src/test/java/com/appsmith/server/services/PageServiceTest.java
View File

@ -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<Application> applicationPageReOrdered = applicationPageService.createPage(testPage1)
Mono<ApplicationPagesDTO> 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<ApplicationPage> pages = application.getPages();
final List<PageNameIdDTO> 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<Application> applicationPageReOrdered = applicationPageService.createPage(testPage1)
Mono<ApplicationPagesDTO> 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<ApplicationPage> pages = application.getPages();
final List<PageNameIdDTO> 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<PageDTO> 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();