From d6670c70cc41282e0a23530cca171047d5a4c8a4 Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Thu, 28 May 2020 13:31:24 +0530 Subject: [PATCH] Bug Root Cause : New users have no applications inside the organizations(s). In this case, get all applications does not return back organizationApplications object. Fix : In getAllApplications, instead of iterating over collections of applications which could be empty, we iterate over organizations where we are guaranteed to have atleast one organization. --- .../services/ApplicationServiceImpl.java | 24 ++++++++------ .../server/configurations/SeedMongoData.java | 4 +-- .../services/ApplicationServiceTest.java | 32 +++++++++++++++++++ 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java index ce0726e75a..6dd9d593f4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ApplicationServiceImpl.java @@ -8,8 +8,8 @@ import com.appsmith.server.domains.ApplicationPage; import com.appsmith.server.domains.Layout; import com.appsmith.server.domains.Organization; import com.appsmith.server.domains.User; -import com.appsmith.server.dtos.UserHomepageDTO; import com.appsmith.server.dtos.OrganizationApplicationsDTO; +import com.appsmith.server.dtos.UserHomepageDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.PolicyUtils; @@ -205,16 +205,20 @@ public class ApplicationServiceImpl extends BaseService> applicationsCollectionByOrgId = tuple.getT1(); Map organizationsMap = tuple.getT2(); - Iterator>> itr = - applicationsCollectionByOrgId.entrySet().iterator(); - List organizationApplicationsDTOS = new ArrayList<>(); - while (itr.hasNext()) { - Map.Entry> next = itr.next(); - String orgId = next.getKey(); - Collection applicationCollection = next.getValue(); - List applicationList = applicationCollection.stream().collect(Collectors.toList()); - Organization organization = organizationsMap.get(orgId); + + Iterator> orgIterator = organizationsMap.entrySet().iterator(); + + while (orgIterator.hasNext()) { + Map.Entry organizationEntry = orgIterator.next(); + String orgId = organizationEntry.getKey(); + Organization organization = organizationEntry.getValue(); + Collection applicationCollection = applicationsCollectionByOrgId.get(orgId); + + List applicationList = null; + if (applicationCollection!=null && !applicationCollection.isEmpty()) { + applicationList = applicationCollection.stream().collect(Collectors.toList()); + } OrganizationApplicationsDTO organizationApplicationsDTO = new OrganizationApplicationsDTO(); organizationApplicationsDTO.setOrganization(organization); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java index 7d2afdaffd..d6e8f99629 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/configurations/SeedMongoData.java @@ -69,7 +69,7 @@ public class SeedMongoData { .build(); Policy userManageOrgPolicy = Policy.builder().permission(USER_MANAGE_ORGANIZATIONS.getValue()) - .users(Set.of(API_USER_EMAIL)) + .users(Set.of(API_USER_EMAIL, TEST_USER_EMAIL)) .build(); Policy managePagePolicy = Policy.builder().permission(MANAGE_PAGES.getValue()) @@ -101,7 +101,7 @@ public class SeedMongoData { .build(); Object[][] userData = { - {"user test", TEST_USER_EMAIL, UserState.ACTIVATED, Set.of(readTestUserPolicy)}, + {"user test", TEST_USER_EMAIL, UserState.ACTIVATED, Set.of(readTestUserPolicy, userManageOrgPolicy)}, {"api_user", API_USER_EMAIL, UserState.ACTIVATED, Set.of(userManageOrgPolicy, readApiUserPolicy, manageApiUserPolicy)}, }; Object[][] orgData = { 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 936800351f..582aa2a45a 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 @@ -3,6 +3,7 @@ package com.appsmith.server.services; import com.appsmith.external.models.Policy; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.Application; +import com.appsmith.server.domains.Organization; import com.appsmith.server.domains.Page; import com.appsmith.server.domains.User; import com.appsmith.server.dtos.OrganizationApplicationsDTO; @@ -51,6 +52,9 @@ public class ApplicationServiceTest { @Autowired UserService userService; + @Autowired + OrganizationService organizationService; + String orgId; @Before @@ -290,4 +294,32 @@ public class ApplicationServiceTest { } + @Test + @WithUserDetails(value = "usertest@usertest.com") + public void getAllApplicationsForHomeWhenNoApplicationPresent() { + // Create an organization for this user first. + Organization organization = new Organization(); + organization.setName("usertest's organization"); + Mono organizationMono = organizationService.create(organization); + + Mono allApplications = organizationMono + .then(applicationService.getAllApplications()); + + StepVerifier + .create(allApplications) + .assertNext(userHomepageDTO -> { + assertThat(userHomepageDTO).isNotNull(); + //In case of anonymous user, we should have errored out. Assert that the user is not anonymous. + assertThat(userHomepageDTO.getUser().getIsAnonymous()).isFalse(); + + List organizationApplications = userHomepageDTO.getOrganizationApplications(); + + // There should be atleast one organization present in the output. + OrganizationApplicationsDTO orgAppDto = organizationApplications.get(0); + assertThat(orgAppDto.getOrganization().getUserPermissions().contains("read:organizations")); + }) + .verifyComplete(); + + } + }