Skip to content

Commit

Permalink
fix: Rename JS Object failing in git child branch (appsmithorg#29174)
Browse files Browse the repository at this point in the history
## Description
Renaming a JS object in child branch fails because the get action
collections by pageId and branch name returns empty list. This PR fixes
this problem.

#### PR fixes following issue(s)
Fixes appsmithorg#28766 appsmithorg#29131
  • Loading branch information
Nayan authored Dec 1, 2023
1 parent b4fd90c commit 4ddb2a4
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,30 @@
*/
@Data
public class DefaultResources {
/**
* When present, actionId will hold the default action id
*/
String actionId;

/**
* When present, applicationId will hold the default application id
*/
String applicationId;

/**
* When present, pageId will hold the default page id
*/
String pageId;

/**
* When present, collectionId will hold the default collection id
*/
String collectionId;

/**
* When present, branchName will hold the current branch name.
* For example, if we've a page in both main and develop branch, then default resources of those two pages will
* have same applicationId, pageId but branchName will contain the corresponding branch name.
*/
String branchName;
}
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ public Flux<ActionCollectionDTO> getActionCollectionsByViewMode(
pageIds.add(params.getFirst(FieldName.PAGE_ID));
}
return repository
.findAllActionCollectionsByNamePageIdsViewModeAndBranch(
.findAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
name, pageIds, viewMode, branch, actionPermission.getReadPermission(), sort)
.flatMap(actionCollection -> generateActionCollectionByViewMode(actionCollection, viewMode));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Flux<ActionCollection> findByApplicationId(
Flux<ActionCollection> findByApplicationIdAndViewMode(
String applicationId, boolean viewMode, AclPermission aclPermission);

Flux<ActionCollection> findAllActionCollectionsByNamePageIdsViewModeAndBranch(
Flux<ActionCollection> findAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
String name,
List<String> pageIds,
boolean viewMode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public Flux<ActionCollection> findByApplicationIdAndViewMode(
}

@Override
public Flux<ActionCollection> findAllActionCollectionsByNamePageIdsViewModeAndBranch(
public Flux<ActionCollection> findAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
String name,
List<String> pageIds,
boolean viewMode,
Expand Down Expand Up @@ -114,9 +114,12 @@ public Flux<ActionCollection> findAllActionCollectionsByNamePageIdsViewModeAndBr
}

if (pageIds != null && !pageIds.isEmpty()) {
Criteria pageCriteria = where(fieldName(QActionCollection.actionCollection.publishedCollection) + "."
+ fieldName(QActionCollection.actionCollection.publishedCollection.pageId))
.in(pageIds);
String pageIdFieldPath = String.format(
"%s.%s.%s",
fieldName(QActionCollection.actionCollection.publishedCollection),
fieldName(QActionCollection.actionCollection.publishedCollection.defaultResources),
fieldName(QActionCollection.actionCollection.publishedCollection.pageId));
Criteria pageCriteria = where(pageIdFieldPath).in(pageIds);
criteriaList.add(pageCriteria);
}
}
Expand All @@ -131,9 +134,12 @@ public Flux<ActionCollection> findAllActionCollectionsByNamePageIdsViewModeAndBr
}

if (pageIds != null && !pageIds.isEmpty()) {
Criteria pageCriteria = where(fieldName(QActionCollection.actionCollection.unpublishedCollection) + "."
+ fieldName(QActionCollection.actionCollection.unpublishedCollection.pageId))
.in(pageIds);
String pageIdFieldPath = String.format(
"%s.%s.%s",
fieldName(QActionCollection.actionCollection.unpublishedCollection),
fieldName(QActionCollection.actionCollection.unpublishedCollection.defaultResources),
fieldName(QActionCollection.actionCollection.unpublishedCollection.pageId));
Criteria pageCriteria = where(pageIdFieldPath).in(pageIds);
criteriaList.add(pageCriteria);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.appsmith.server.repositories.ce;

import com.appsmith.external.models.DefaultResources;
import com.appsmith.server.domains.ActionCollection;
import com.appsmith.server.dtos.ActionCollectionDTO;
import com.appsmith.server.repositories.ActionCollectionRepository;
import org.bson.types.ObjectId;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -102,4 +104,69 @@ public void bulkInsert_WhenInsertedWithProvidedId_InsertedWithProvidedId() {
})
.verifyComplete();
}

private void testFindAllActionCollectionsByNamePageIdsViewModeAndBranch(boolean isViewMode) {
String defaultPageId = "default-page-id", branchName = "main", childPageId = "child-page-id";

// create action collection that has different values in pageId and defaultResources.pageId
ActionCollection actionCollection = new ActionCollection();
ActionCollectionDTO actionCollectionDTO = new ActionCollectionDTO();
actionCollectionDTO.setPageId(childPageId);
actionCollectionDTO.setDefaultResources(new DefaultResources());
actionCollectionDTO.getDefaultResources().setPageId(defaultPageId);

if (isViewMode) {
actionCollection.setPublishedCollection(actionCollectionDTO);
} else {
actionCollection.setUnpublishedCollection(actionCollectionDTO);
}

actionCollection.setDefaultResources(new DefaultResources());
actionCollection.getDefaultResources().setBranchName(branchName);

Mono<ActionCollection> createActionCollectionMono =
actionCollectionRepository.save(actionCollection).cache();

// check whether action collection is found when branch and default page id matches
Mono<List<ActionCollection>> actionCollectionListMono = createActionCollectionMono
.thenMany(actionCollectionRepository.findAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
null, List.of(defaultPageId), isViewMode, branchName, null, null))
.collectList();

StepVerifier.create(actionCollectionListMono)
.assertNext(actionCollectionList -> {
assertThat(actionCollectionList.size()).isEqualTo(1);
})
.verifyComplete();

// check whether action collection is not found when branch name does not match
Mono<List<ActionCollection>> actionCollectionListMono2 = createActionCollectionMono
.thenMany(actionCollectionRepository.findAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
null, List.of(defaultPageId), isViewMode, "feature", null, null))
.collectList();

StepVerifier.create(actionCollectionListMono2)
.assertNext(actionCollectionList -> {
assertThat(actionCollectionList.size()).isEqualTo(0);
})
.verifyComplete();

// check whether action collection is not found when default page id does not match
Mono<List<ActionCollection>> actionCollectionListMono3 = createActionCollectionMono
.thenMany(actionCollectionRepository.findAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
null, List.of(childPageId), isViewMode, branchName, null, null))
.collectList();

StepVerifier.create(actionCollectionListMono3)
.assertNext(actionCollectionList -> {
assertThat(actionCollectionList.size()).isEqualTo(0);
})
.verifyComplete();
}

@Test
public void findAllActionCollectionsByNamePageIdsViewModeAndBranch_ForChildBranch_Successful() {
testFindAllActionCollectionsByNamePageIdsViewModeAndBranch(false);
testFindAllActionCollectionsByNamePageIdsViewModeAndBranch(true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ public void testCreateCollection_withRepeatedActionName_throwsError() throws IOE
Mockito.when(refactoringService.isNameAllowed(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()))
.thenReturn(Mono.just(false));

Mockito.when(actionCollectionRepository.findAllActionCollectionsByNamePageIdsViewModeAndBranch(
Mockito.when(actionCollectionRepository.findAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
Mockito.any(),
Mockito.any(),
Mockito.anyBoolean(),
Expand Down Expand Up @@ -278,7 +278,7 @@ public void testCreateCollection_createActionFailure_returnsWithIncompleteCollec
Mockito.when(refactoringService.isNameAllowed(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()))
.thenReturn(Mono.just(true));

Mockito.when(actionCollectionRepository.findAllActionCollectionsByNamePageIdsViewModeAndBranch(
Mockito.when(actionCollectionRepository.findAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
Mockito.any(),
Mockito.any(),
Mockito.anyBoolean(),
Expand Down Expand Up @@ -343,7 +343,7 @@ public void testCreateCollection_validCollection_returnsPopulatedCollection() th
Mockito.when(refactoringService.isNameAllowed(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()))
.thenReturn(Mono.just(true));

Mockito.when(actionCollectionRepository.findAllActionCollectionsByNamePageIdsViewModeAndBranch(
Mockito.when(actionCollectionRepository.findAllActionCollectionsByNameDefaultPageIdsViewModeAndBranch(
Mockito.any(),
Mockito.any(),
Mockito.anyBoolean(),
Expand Down

0 comments on commit 4ddb2a4

Please sign in to comment.