fix: Update ApplicationPageId with defaultPageId while fetching applications for homepage (#10812)

This commit is contained in:
Abhijeet 2022-02-03 10:56:35 +05:30 committed by GitHub
parent 743d6cb00d
commit 0b1a8ad20b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 122 additions and 16 deletions

View File

@ -7,7 +7,6 @@ import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import lombok.ToString;
import org.springframework.data.annotation.Transient;
import org.springframework.data.mongodb.core.mapping.Document;
import javax.validation.constraints.NotBlank;
@ -55,7 +54,4 @@ public class Organization extends BaseDomain {
return Url.ASSET_URL + "/" + logoAssetId;
}
@Transient
private long gitConnectedApplications;
}

View File

@ -327,6 +327,15 @@ public class ResponseUtils {
}
});
}
if (!CollectionUtils.isEmpty(application.getPublishedPages())) {
application
.getPublishedPages()
.forEach(page -> {
if (!StringUtils.isEmpty(page.getDefaultPageId())) {
page.setId(page.getDefaultPageId());
}
});
}
return application;
}

View File

@ -1,5 +1,6 @@
package com.appsmith.server.solutions;
import com.appsmith.server.helpers.ResponseUtils;
import com.appsmith.server.repositories.ApplicationRepository;
import com.appsmith.server.services.OrganizationService;
import com.appsmith.server.services.SessionUserService;
@ -17,9 +18,10 @@ public class ApplicationFetcherImpl extends ApplicationFetcherCEImpl implements
UserDataService userDataService,
OrganizationService organizationService,
ApplicationRepository applicationRepository,
ReleaseNotesService releaseNotesService) {
ReleaseNotesService releaseNotesService,
ResponseUtils responseUtils) {
super(sessionUserService, userService, userDataService, organizationService, applicationRepository,
releaseNotesService);
releaseNotesService, responseUtils);
}
}

View File

@ -9,6 +9,7 @@ import com.appsmith.server.dtos.ReleaseNode;
import com.appsmith.server.dtos.UserHomepageDTO;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.ResponseUtils;
import com.appsmith.server.repositories.ApplicationRepository;
import com.appsmith.server.services.OrganizationService;
import com.appsmith.server.services.SessionUserService;
@ -50,6 +51,7 @@ public class ApplicationFetcherCEImpl implements ApplicationFetcherCE {
private final OrganizationService organizationService;
private final ApplicationRepository applicationRepository;
private final ReleaseNotesService releaseNotesService;
private final ResponseUtils responseUtils;
/**
* For the current user, it first fetches all the organizations that its part of. For each organization, in turn all
@ -100,7 +102,9 @@ public class ApplicationFetcherCEImpl implements ApplicationFetcherCE {
Flux<Application> applicationFlux = applicationRepository
.findByMultipleOrganizationIds(orgIds, READ_APPLICATIONS)
.filter(application -> application.getGitApplicationMetadata() == null
|| (StringUtils.equals(application.getId(), application.getGitApplicationMetadata().getDefaultApplicationId())));
|| (StringUtils.equals(application.getId(), application.getGitApplicationMetadata().getDefaultApplicationId()))
)
.map(responseUtils::updateApplicationWithDefaultResources);
// sort the list of applications if user has recent applications
if(!CollectionUtils.isEmpty(userData.getRecentlyUsedAppIds())) {
@ -140,11 +144,6 @@ public class ApplicationFetcherCEImpl implements ApplicationFetcherCE {
final List<Application> applicationList = new ArrayList<>();
if (!CollectionUtils.isEmpty(applicationCollection)) {
applicationList.addAll(applicationCollection);
long gitConnectedAppsCount = applicationCollection
.stream()
.filter(application -> application.getGitApplicationMetadata() != null)
.count();
organization.setGitConnectedApplications(gitConnectedAppsCount);
}
OrganizationApplicationsDTO organizationApplicationsDTO = new OrganizationApplicationsDTO();

View File

@ -2,6 +2,8 @@ package com.appsmith.server.helpers;
import com.appsmith.external.helpers.BeanCopyUtils;
import com.appsmith.server.domains.ActionCollection;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.ApplicationPage;
import com.appsmith.server.domains.NewAction;
import com.appsmith.server.domains.NewPage;
import com.fasterxml.jackson.databind.JsonNode;
@ -103,4 +105,29 @@ public class ResponseUtilsTest {
Assert.assertEquals(actionCollectionCopy.getUnpublishedCollection().getDefaultResources().getPageId(), actionCollection.getUnpublishedCollection().getPageId());
Assert.assertEquals(actionCollectionCopy.getPublishedCollection().getDefaultResources().getPageId(), actionCollection.getPublishedCollection().getPageId());
}
@Test
public void getApplication_withDefaultIdsPresent_returnsUpdatedApplication() {
Application application = gson.fromJson(String.valueOf(jsonNode.get("application")), Application.class);
final Application applicationCopy = new Application();
BeanCopyUtils.copyNestedNonNullProperties(application, applicationCopy);
// Check if the id and defaultPage ids for pages are not same before we update the application using responseUtils
for (ApplicationPage applicationPage : application.getPages()) {
Assert.assertNotEquals(applicationPage.getId(), applicationPage.getDefaultPageId());
}
for (ApplicationPage applicationPage : application.getPublishedPages()) {
Assert.assertNotEquals(applicationPage.getId(), applicationPage.getDefaultPageId());
}
responseUtils.updateApplicationWithDefaultResources(application);
Assert.assertNotEquals(applicationCopy, application);
// Check if the id and defaultPage ids for pages are same after we update the application from responseUtils
for (ApplicationPage applicationPage : application.getPages()) {
Assert.assertEquals(applicationPage.getId(), applicationPage.getDefaultPageId());
}
for (ApplicationPage applicationPage : application.getPublishedPages()) {
Assert.assertEquals(applicationPage.getId(), applicationPage.getDefaultPageId());
}
}
}

View File

@ -1,10 +1,12 @@
package com.appsmith.server.solutions;
import com.appsmith.server.domains.Application;
import com.appsmith.server.domains.ApplicationPage;
import com.appsmith.server.domains.Organization;
import com.appsmith.server.domains.User;
import com.appsmith.server.domains.UserData;
import com.appsmith.server.dtos.OrganizationApplicationsDTO;
import com.appsmith.server.helpers.ResponseUtils;
import com.appsmith.server.repositories.ApplicationRepository;
import com.appsmith.server.services.OrganizationService;
import com.appsmith.server.services.SessionUserService;
@ -49,10 +51,14 @@ public class ApplicationFetcherUnitTest {
@MockBean
ReleaseNotesService releaseNotesService;
@MockBean
ResponseUtils responseUtils;
ApplicationFetcher applicationFetcher;
User testUser;
final static String defaultPageId = "defaultPageId";
@Before
public void setup() {
@ -61,7 +67,8 @@ public class ApplicationFetcherUnitTest {
userDataService,
organizationService,
applicationRepository,
releaseNotesService);
releaseNotesService,
responseUtils);
}
private List<Application> createDummyApplications(int orgCount, int appCount) {
@ -72,12 +79,24 @@ public class ApplicationFetcherUnitTest {
application.setOrganizationId("org-" + i);
application.setId("org-" + i + "-app-" + j); // e.g. org-1-app-3
application.setName(application.getId()); // e.g. org-1-app-3
// Set dummy applicationPages
ApplicationPage applicationPage = new ApplicationPage();
applicationPage.setId("page" + j);
applicationPage.setDefaultPageId(defaultPageId);
application.setPages(List.of(applicationPage));
application.setPublishedPages(List.of(applicationPage));
applicationList.add(application);
}
}
return applicationList;
}
private Application updateDefaultPageIdsWithinApplication(Application application) {
application.getPublishedPages().forEach(page -> page.setId(page.getDefaultPageId()));
application.getPages().forEach(page -> page.setId(page.getDefaultPageId()));
return application;
}
private List<Organization> createDummyOrganizations() {
List<Organization> organizationList = new ArrayList<>(4);
for(int i = 1; i <= 4; i++) {
@ -112,9 +131,16 @@ public class ApplicationFetcherUnitTest {
Mockito.when(userDataService.getForCurrentUser()).thenReturn(Mono.just(userData));
// mock the list of applications
List<Application> applications = createDummyApplications(4,4);
Mockito.when(applicationRepository.findByMultipleOrganizationIds(
testUser.getOrganizationIds(), READ_APPLICATIONS)
).thenReturn(Flux.fromIterable(createDummyApplications(4,4)));
).thenReturn(Flux.fromIterable(applications));
for (Application application : applications) {
Mockito
.when(responseUtils.updateApplicationWithDefaultResources(application))
.thenReturn(updateDefaultPageIdsWithinApplication(application));
}
StepVerifier.create(applicationFetcher.getAllApplications())
.assertNext(userHomepageDTO -> {
@ -122,6 +148,14 @@ public class ApplicationFetcherUnitTest {
assertThat(dtos.size()).isEqualTo(4);
for (OrganizationApplicationsDTO dto : dtos) {
assertThat(dto.getApplications().size()).isEqualTo(4);
List<Application> applicationList = dto.getApplications();
for (Application application : applicationList) {
List<ApplicationPage> pages = application.getPages();
pages.forEach(page -> assertThat(page.getId()).isEqualTo(defaultPageId));
pages = application.getPublishedPages();
pages.forEach(page -> assertThat(page.getId()).isEqualTo(defaultPageId));
}
}
}).verifyComplete();
}
@ -136,9 +170,15 @@ public class ApplicationFetcherUnitTest {
Mockito.when(userDataService.getForCurrentUser()).thenReturn(Mono.just(userData));
// mock the list of applications
List<Application> applications = createDummyApplications(4,4);
Mockito.when(applicationRepository.findByMultipleOrganizationIds(
testUser.getOrganizationIds(), READ_APPLICATIONS)
).thenReturn(Flux.fromIterable(createDummyApplications(4,4)));
).thenReturn(Flux.fromIterable(applications));
for (Application application : applications) {
Mockito
.when(responseUtils.updateApplicationWithDefaultResources(application))
.thenReturn(updateDefaultPageIdsWithinApplication(application));
}
StepVerifier.create(applicationFetcher.getAllApplications())
.assertNext(userHomepageDTO -> {
@ -177,9 +217,15 @@ public class ApplicationFetcherUnitTest {
Mockito.when(userDataService.getForCurrentUser()).thenReturn(Mono.just(userData));
// mock the list of applications
List<Application> applications = createDummyApplications(3,3);
Mockito.when(applicationRepository.findByMultipleOrganizationIds(
testUser.getOrganizationIds(), READ_APPLICATIONS)
).thenReturn(Flux.fromIterable(createDummyApplications(3,3)));
).thenReturn(Flux.fromIterable(applications));
for (Application application : applications) {
Mockito
.when(responseUtils.updateApplicationWithDefaultResources(application))
.thenReturn(updateDefaultPageIdsWithinApplication(application));
}
StepVerifier.create(applicationFetcher.getAllApplications())
.assertNext(userHomepageDTO -> {

View File

@ -51,5 +51,32 @@
"collectionId": "defaultTestCollectionId",
"applicationId": "defaultApplicationId"
}
},
"application": {
"id": "testApplicationId",
"pages": [
{
"id" : "testPageId1",
"isDefault": true,
"defaultPageId": "defaultTestPageId1"
},
{
"id" : "testPageId2",
"isDefault": false,
"defaultPageId": "defaultTestPageId2"
}
],
"publishedPages": [
{
"id" : "testPageId1",
"isDefault": false,
"defaultPageId": "defaultTestPageId1"
},
{
"id" : "testPageId2",
"isDefault": true,
"defaultPageId": "defaultTestPageId2"
}
]
}
}