From 36c399ac4f4d4e9435de0f41f9c27012a089ca65 Mon Sep 17 00:00:00 2001 From: Nayan <83352306+nayan-rafiq@users.noreply.github.com> Date: Mon, 28 Jun 2021 14:50:39 +0600 Subject: [PATCH] Fix the comment missing issue when new users added to organization (#5314) * -Add the newly added user to existing comment threads * -update PR as per review comment * -updated the PR as per review --- .../appsmith/server/helpers/PolicyUtils.java | 36 ++++++++++++------- .../CustomCommentThreadRepositoryImpl.java | 1 - .../services/UserOrganizationServiceImpl.java | 34 ++++++++++++++---- .../server/services/UserServiceImpl.java | 1 - .../server/solutions/UserChangedHandler.java | 1 - 5 files changed, 51 insertions(+), 22 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/PolicyUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/PolicyUtils.java index 7c88e95432..979345fa3d 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/PolicyUtils.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/PolicyUtils.java @@ -5,14 +5,18 @@ import com.appsmith.external.models.Policy; import com.appsmith.server.acl.AclPermission; import com.appsmith.server.acl.PolicyGenerator; import com.appsmith.server.domains.Application; +import com.appsmith.server.domains.CommentThread; import com.appsmith.server.domains.Datasource; import com.appsmith.server.domains.NewAction; import com.appsmith.server.domains.NewPage; import com.appsmith.server.domains.User; import com.appsmith.server.repositories.ApplicationRepository; +import com.appsmith.server.repositories.CommentThreadRepository; import com.appsmith.server.repositories.DatasourceRepository; import com.appsmith.server.repositories.NewActionRepository; import com.appsmith.server.repositories.NewPageRepository; +import com.appsmith.server.solutions.UserChangedHandler; +import lombok.AllArgsConstructor; import org.apache.commons.collections.CollectionUtils; import org.springframework.stereotype.Component; import reactor.core.publisher.Flux; @@ -32,6 +36,7 @@ import java.util.stream.Collectors; import static com.appsmith.server.acl.AclPermission.MANAGE_DATASOURCES; @Component +@AllArgsConstructor public class PolicyUtils { private final PolicyGenerator policyGenerator; @@ -39,18 +44,8 @@ public class PolicyUtils { private final DatasourceRepository datasourceRepository; private final NewPageRepository newPageRepository; private final NewActionRepository newActionRepository; - - public PolicyUtils(PolicyGenerator policyGenerator, - ApplicationRepository applicationRepository, - DatasourceRepository datasourceRepository, - NewPageRepository newPageRepository, - NewActionRepository newActionRepository) { - this.policyGenerator = policyGenerator; - this.applicationRepository = applicationRepository; - this.datasourceRepository = datasourceRepository; - this.newPageRepository = newPageRepository; - this.newActionRepository = newActionRepository; - } + private final UserChangedHandler userChangedHandler; + private final CommentThreadRepository commentThreadRepository; public T addPoliciesToExistingObject(Map policyMap, T obj) { // Making a deep copy here so we don't modify the `policyMap` object. @@ -224,6 +219,23 @@ public class PolicyUtils { .saveAll(updatedPages)); } + public Flux updateWithApplicationPermissionsToAllItsCommentThreads(String applicationId, Map commentThreadPolicyMap, boolean addPolicyToObject) { + + return + // fetch comment threads with read permissions + commentThreadRepository.findByApplicationId(applicationId, AclPermission.READ_THREAD) + .switchIfEmpty(Mono.empty()) + .map(thread -> { + if (addPolicyToObject) { + return addPoliciesToExistingObject(commentThreadPolicyMap, thread); + } else { + return removePoliciesFromExistingObject(commentThreadPolicyMap, thread); + } + }) + .collectList() + .flatMapMany(commentThreads -> commentThreadRepository.saveAll(commentThreads)); + } + /** * Instead of fetching actions by pageId, fetch actions by applicationId and then update the action policies * using the new ActionPoliciesMap. This ensures the following : diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomCommentThreadRepositoryImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomCommentThreadRepositoryImpl.java index 16e28f9984..63fdf0d2ed 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomCommentThreadRepositoryImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomCommentThreadRepositoryImpl.java @@ -46,5 +46,4 @@ public class CustomCommentThreadRepositoryImpl extends BaseAppsmithRepositoryImp CommentThread.class ); } - } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserOrganizationServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserOrganizationServiceImpl.java index f6cb8ddbf0..2369927505 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserOrganizationServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserOrganizationServiceImpl.java @@ -6,6 +6,7 @@ import com.appsmith.server.acl.AppsmithRole; import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.Action; import com.appsmith.server.domains.Application; +import com.appsmith.server.domains.CommentThread; import com.appsmith.server.domains.Datasource; import com.appsmith.server.domains.NewAction; import com.appsmith.server.domains.NewPage; @@ -240,6 +241,9 @@ public class UserOrganizationServiceImpl implements UserOrganizationService { Map datasourcePolicyMap = policyUtils.generateInheritedPoliciesFromSourcePolicies(orgPolicyMap, Organization.class, Datasource.class); Map pagePolicyMap = policyUtils.generateInheritedPoliciesFromSourcePolicies(applicationPolicyMap, Application.class, Page.class); Map actionPolicyMap = policyUtils.generateInheritedPoliciesFromSourcePolicies(pagePolicyMap, Page.class, Action.class); + Map commentThreadPolicyMap = policyUtils.generateInheritedPoliciesFromSourcePolicies( + applicationPolicyMap, Application.class, CommentThread.class + ); //Now update the organization policies Organization updatedOrganization = policyUtils.removePoliciesFromExistingObject(orgPolicyMap, organization); @@ -253,13 +257,20 @@ public class UserOrganizationServiceImpl implements UserOrganizationService { .flatMap(application -> policyUtils.updateWithApplicationPermissionsToAllItsPages(application.getId(), pagePolicyMap, false)); Flux updatedActionsFlux = updatedApplicationsFlux .flatMap(application -> policyUtils.updateWithPagePermissionsToAllItsActions(application.getId(), actionPolicyMap, false)); + Flux updatedThreadsFlux = updatedApplicationsFlux + .flatMap(application -> policyUtils.updateWithApplicationPermissionsToAllItsCommentThreads(application.getId(), commentThreadPolicyMap, false)); - return Mono.zip(updatedDatasourcesFlux.collectList(), updatedPagesFlux.collectList(), updatedActionsFlux.collectList(), Mono.just(updatedOrganization)) - .flatMap(tuple -> { - //By now all the datasources/applications/pages/actions have been updated. Just save the organization now - Organization updatedOrgBeforeSave = tuple.getT4(); - return organizationRepository.save(updatedOrgBeforeSave); - }); + return Mono.zip( + updatedDatasourcesFlux.collectList(), + updatedPagesFlux.collectList(), + updatedActionsFlux.collectList(), + updatedThreadsFlux.collectList(), + Mono.just(updatedOrganization) + ).flatMap(tuple -> { + //By now all the datasources/applications/pages/actions have been updated. Just save the organization now + Organization updatedOrgBeforeSave = tuple.getT5(); + return organizationRepository.save(updatedOrgBeforeSave); + }); } private Mono updateMemberRole(Organization organization, User user, UserRole userRole, User currentUser, String originHeader) { @@ -403,6 +414,9 @@ public class UserOrganizationServiceImpl implements UserOrganizationService { Map applicationPolicyMap = policyUtils.generateInheritedPoliciesFromSourcePolicies(orgPolicyMap, Organization.class, Application.class); Map datasourcePolicyMap = policyUtils.generateInheritedPoliciesFromSourcePolicies(orgPolicyMap, Organization.class, Datasource.class); Map pagePolicyMap = policyUtils.generateInheritedPoliciesFromSourcePolicies(applicationPolicyMap, Application.class, Page.class); + Map commentThreadPolicyMap = policyUtils.generateInheritedPoliciesFromSourcePolicies( + applicationPolicyMap, Application.class, CommentThread.class + ); Map actionPolicyMap = policyUtils.generateInheritedPoliciesFromSourcePolicies(pagePolicyMap, Page.class, Action.class); //Now update the organization policies @@ -417,8 +431,14 @@ public class UserOrganizationServiceImpl implements UserOrganizationService { .flatMap(application -> policyUtils.updateWithApplicationPermissionsToAllItsPages(application.getId(), pagePolicyMap, true)); Flux updatedActionsFlux = updatedApplicationsFlux .flatMap(application -> policyUtils.updateWithPagePermissionsToAllItsActions(application.getId(), actionPolicyMap, true)); + Flux updatedThreadsFlux = updatedApplicationsFlux + .flatMap(application -> policyUtils.updateWithApplicationPermissionsToAllItsCommentThreads(application.getId(), commentThreadPolicyMap, true)); - return Mono.when(updatedDatasourcesFlux.collectList(), updatedPagesFlux.collectList(), updatedActionsFlux.collectList()) + return Mono.when( + updatedDatasourcesFlux.collectList(), + updatedPagesFlux.collectList(), + updatedActionsFlux.collectList(), + updatedThreadsFlux.collectList()) //By now all the datasources/applications/pages/actions have been updated. Just save the organization now .then(organizationRepository.save(updatedOrganization)); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java index 5e20ae3d96..8d1ad1b3c4 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserServiceImpl.java @@ -623,7 +623,6 @@ public class UserServiceImpl extends BaseService i .flatMap(tuple -> { List invitedUsers = tuple.getT1(); Organization organization = tuple.getT2(); - return userOrganizationService.bulkAddUsersToOrganization(organization, invitedUsers, inviteUsersDTO.getRoleName()); }); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserChangedHandler.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserChangedHandler.java index 8fa28c55f9..31432fbb9c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserChangedHandler.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/UserChangedHandler.java @@ -19,7 +19,6 @@ import reactor.core.scheduler.Schedulers; public class UserChangedHandler { private final ApplicationEventPublisher applicationEventPublisher; - private final CommentRepository commentRepository; private final OrganizationRepository organizationRepository;