[Issue #3785][Bug] User names in organization management page are stale (#4702)

* [Issue #3785][Bug] User names in organization management page are stale

* -update name in userroles in background when there is a change in user
This commit is contained in:
Nayan 2021-06-02 15:56:52 +06:00 committed by GitHub
parent 78915301e3
commit 77940c0cfa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 212 additions and 0 deletions

View File

@ -16,4 +16,5 @@ public interface CustomOrganizationRepository extends AppsmithRepository<Organiz
Mono<Long> nextSlugNumber(String slugPrefix);
Mono<Void> updateUserRoleNames(String userId, String userName);
}

View File

@ -1,6 +1,7 @@
package com.appsmith.server.repositories;
import com.appsmith.server.acl.AclPermission;
import com.appsmith.server.domains.Comment;
import com.appsmith.server.domains.Organization;
import com.appsmith.server.domains.QOrganization;
import lombok.extern.slf4j.Slf4j;
@ -9,6 +10,7 @@ import org.springframework.data.mongodb.core.ReactiveMongoOperations;
import org.springframework.data.mongodb.core.convert.MongoConverter;
import org.springframework.data.mongodb.core.query.Criteria;
import org.springframework.data.mongodb.core.query.Query;
import org.springframework.data.mongodb.core.query.Update;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
@ -66,4 +68,14 @@ public class CustomOrganizationRepositoryImpl extends BaseAppsmithRepositoryImpl
});
}
@Override
public Mono<Void> updateUserRoleNames(String userId, String userName) {
return mongoOperations
.updateMulti(
Query.query(Criteria.where("userRoles.userId").is(userId)),
Update.update("userRoles.$.name", userName),
Organization.class
)
.then();
}
}

View File

@ -13,4 +13,6 @@ public interface OrganizationRepository extends BaseRepository<Organization, Str
Mono<Organization> findByName(String name);
Mono<Void> updateUserRoleNames(String userId, String userName);
}

View File

@ -1,5 +1,6 @@
package com.appsmith.server.services;
import com.appsmith.external.models.BaseDomain;
import com.appsmith.server.acl.AclPermission;
import com.appsmith.server.acl.AppsmithRole;
import com.appsmith.server.acl.RoleGraph;
@ -35,6 +36,7 @@ import javax.validation.Validator;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.ArrayList;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

View File

@ -3,6 +3,7 @@ package com.appsmith.server.solutions;
import com.appsmith.server.domains.User;
import com.appsmith.server.events.UserChangedEvent;
import com.appsmith.server.repositories.CommentRepository;
import com.appsmith.server.repositories.OrganizationRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.ApplicationEventPublisher;
@ -20,6 +21,7 @@ public class UserChangedHandler {
private final ApplicationEventPublisher applicationEventPublisher;
private final CommentRepository commentRepository;
private final OrganizationRepository organizationRepository;
public User publish(User user) {
applicationEventPublisher.publishEvent(new UserChangedEvent(user));
@ -35,6 +37,10 @@ public class UserChangedHandler {
updateNameInComments(user)
.subscribeOn(Schedulers.elastic())
.subscribe();
updateNameInUserRoles(user)
.subscribeOn(Schedulers.elastic())
.subscribe();
}
private Mono<Void> updateNameInComments(User user) {
@ -47,4 +53,13 @@ public class UserChangedHandler {
return commentRepository.updateAuthorNames(user.getId(), user.getName());
}
private Mono<Void> updateNameInUserRoles(User user) {
if (user.getId() == null) {
log.warn("Attempt to update name in userRoles of organization for user with null ID.");
return Mono.empty();
}
log.debug("Updating name in userRoles of organization for user {}", user.getId());
return organizationRepository.updateUserRoleNames(user.getId(), user.getName());
}
}

View File

@ -0,0 +1,83 @@
package com.appsmith.server.repositories;
import com.appsmith.server.domains.Organization;
import com.appsmith.server.domains.UserRole;
import lombok.extern.slf4j.Slf4j;
import org.junit.Assert;
import org.junit.jupiter.api.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.junit4.SpringRunner;
import reactor.core.publisher.Mono;
import reactor.test.StepVerifier;
import reactor.util.function.Tuple2;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import static org.junit.jupiter.api.Assertions.*;
@RunWith(SpringRunner.class)
@SpringBootTest
@Slf4j
@DirtiesContext
class OrganizationRepositoryTest {
@Autowired
private OrganizationRepository organizationRepository;
@Test
void updateUserRoleNames_WhenUserIdMatched_AllOrgsUpdated() {
String oldUserName = "Old name",
newUserName = "New name",
userId = "user1";
UserRole userRole = new UserRole();
userRole.setName(oldUserName);
userRole.setUserId(userId);
List<UserRole> userRoles = new ArrayList<>();
userRoles.add(userRole);
Organization org1 = new Organization();
org1.setId(UUID.randomUUID().toString());
org1.setSlug(org1.getId());
org1.setUserRoles(userRoles);
Organization org2 = new Organization();
org2.setId(UUID.randomUUID().toString());
org2.setSlug(org2.getId());
org2.setUserRoles(userRoles);
// create two orgs
Mono<Tuple2<Organization, Organization>> aveOrgsMonoZip = Mono.zip(
organizationRepository.save(org1), organizationRepository.save(org2)
);
Mono<Tuple2<Organization, Organization>> updatedOrgTupleMono = aveOrgsMonoZip.flatMap(objects -> {
// update the user names
return organizationRepository.updateUserRoleNames(userId, newUserName).thenReturn(objects);
}).flatMap(organizationTuple2 -> {
// fetch the two orgs again
Mono<Organization> updatedOrg1Mono = organizationRepository.findBySlug(org1.getId());
Mono<Organization> updatedOrg2Mono = organizationRepository.findBySlug(org2.getId());
return Mono.zip(updatedOrg1Mono, updatedOrg2Mono);
});
StepVerifier.create(updatedOrgTupleMono).assertNext(orgTuple -> {
Organization o1 = orgTuple.getT1();
Assert.assertEquals(1, o1.getUserRoles().size());
UserRole userRole1 = o1.getUserRoles().get(0);
Assert.assertEquals(userId, userRole1.getUserId());
Assert.assertEquals(newUserName, userRole1.getName());
Organization o2 = orgTuple.getT2();
Assert.assertEquals(1, o2.getUserRoles().size());
UserRole userRole2 = o2.getUserRoles().get(0);
Assert.assertEquals(userId, userRole2.getUserId());
Assert.assertEquals(newUserName, userRole2.getName());
}).verifyComplete();
}
}

View File

@ -0,0 +1,97 @@
package com.appsmith.server.services;
import com.appsmith.server.acl.RoleGraph;
import com.appsmith.server.constants.FieldName;
import com.appsmith.server.domains.Organization;
import com.appsmith.server.domains.User;
import com.appsmith.server.domains.UserRole;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.repositories.AssetRepository;
import com.appsmith.server.repositories.OrganizationRepository;
import com.appsmith.server.repositories.PluginRepository;
import com.appsmith.server.repositories.UserRepository;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.data.mongodb.core.ReactiveMongoTemplate;
import org.springframework.data.mongodb.core.convert.MongoConverter;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
import reactor.core.scheduler.Scheduler;
import reactor.test.StepVerifier;
import javax.validation.Validator;
import java.util.ArrayList;
import java.util.List;
import static com.appsmith.server.acl.AclPermission.ORGANIZATION_INVITE_USERS;
@RunWith(SpringJUnit4ClassRunner.class)
public class OrganizationServiceUnitTest {
@MockBean PluginRepository pluginRepository;
@MockBean SessionUserService sessionUserService;
@MockBean UserOrganizationService userOrganizationService;
@MockBean UserRepository userRepository;
@MockBean RoleGraph roleGraph;
@MockBean AssetRepository assetRepository;
@MockBean AssetService assetService;
@MockBean Scheduler scheduler;
@MockBean MongoConverter mongoConverter;
@MockBean ReactiveMongoTemplate reactiveMongoTemplate;
@MockBean OrganizationRepository organizationRepository;
@MockBean Validator validator;
@MockBean AnalyticsService analyticsService;
OrganizationService organizationService;
@Before
public void setUp() {
organizationService = new OrganizationServiceImpl(scheduler, validator, mongoConverter, reactiveMongoTemplate,
organizationRepository, analyticsService, pluginRepository, sessionUserService, userOrganizationService,
userRepository, roleGraph, assetRepository, assetService
);
MockitoAnnotations.initMocks(this);
}
@Test
public void getOrganizationMembers_WhenRoleIsNull_ReturnsEmptyList() {
// create a organization object
Organization testOrg = new Organization();
testOrg.setName("Get All Members For Organization Test");
testOrg.setDomain("test.com");
testOrg.setWebsite("https://test.com");
testOrg.setId("test-org-id");
// mock repository methods so that they return the objects we've created
Mockito.when(organizationRepository.findById("test-org-id", ORGANIZATION_INVITE_USERS))
.thenReturn(Mono.just(testOrg));
Mono<List<UserRole>> organizationMembers = organizationService.getOrganizationMembers(testOrg.getId());
StepVerifier
.create(organizationMembers)
.assertNext(userRoles -> {
Assert.assertEquals(0, userRoles.size());
})
.verifyComplete();
}
@Test
public void getOrganizationMembers_WhenNoOrgFound_ThrowsException() {
String sampleOrgId = "test-org-id";
// mock repository methods so that they return the objects we've created
Mockito.when(organizationRepository.findById(sampleOrgId, ORGANIZATION_INVITE_USERS))
.thenReturn(Mono.empty());
Mono<List<UserRole>> organizationMembers = organizationService.getOrganizationMembers(sampleOrgId);
StepVerifier
.create(organizationMembers)
.expectErrorMessage(AppsmithError.NO_RESOURCE_FOUND.getMessage(FieldName.ORGANIZATION, sampleOrgId))
.verify();
}
}