Skip to content

Commit

Permalink
Merge branch 'develop' into PEPPER-947
Browse files Browse the repository at this point in the history
  • Loading branch information
@pegahtah
pegahtah authored Aug 3, 2023
2 parents c25f895 + 4488d39 commit 590a8c0
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 34 deletions.
15 changes: 1 addition & 14 deletions ...r-apis/dsm-core/src/main/java/org/broadinstitute/dsm/service/admin/UpdateUserRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public UpdateUserRequest(List<UpdateUserRequest.User> users) {

@Data
public static class User {
private final String email;
private String email;
private final String name;
private final String phone;

Expand All @@ -31,18 +31,5 @@ public User(String email, String name, String phone) {
public UserDto asUserDto() {
return new UserDto(name, email, phone);
}

public UserDto asUpdatedUserDto(UserDto userDto) {
if (!email.equalsIgnoreCase(userDto.getEmailOrThrow())) {
throw new DsmInternalError("Assert: email addresses do not match");
}
if (!StringUtils.isBlank(name)) {
userDto.setName(name);
}
if (!StringUtils.isBlank(phone)) {
userDto.setPhoneNumber(phone);
}
return userDto;
}
}
}
69 changes: 50 additions & 19 deletions ...er-apis/dsm-core/src/main/java/org/broadinstitute/dsm/service/admin/UserAdminService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ public class UserAdminService {
private final String studyGroup;
protected static final String USER_ADMIN_ROLE = "study_user_admin";
protected static final String PEPPER_ADMIN_ROLE = "pepper_admin";
private int adminId;
private String adminEmail;
private boolean initialized;

private static final String SQL_SELECT_STUDY_GROUP =
"SELECT dg.group_id FROM ddp_group dg WHERE dg.name = ?";
Expand Down Expand Up @@ -111,12 +114,30 @@ public class UserAdminService {
public UserAdminService(String operatorId, String studyGroup) {
this.operatorId = operatorId;
this.studyGroup = studyGroup;
initialized = false;
}

protected void initialize() {
if (initialized) {
return;
}
try {
adminId = Integer.parseInt(operatorId);
} catch (NumberFormatException e) {
throw new DSMBadRequestException("Invalid operator ID format: " + operatorId);
}
UserDao userDao = new UserDao();
UserDto userDto = userDao.get(adminId).orElseThrow(() -> new DSMBadRequestException("Invalid operator ID " + operatorId));
adminEmail = userDto.getEmailOrThrow();

initialized = true;
}

/**
* Get study roles that operator can administer
*/
public StudyRoleResponse getStudyRoles() {
initialize();
int groupId = verifyStudyGroup(studyGroup);
Map<String, RoleInfo> studyRoles = getAdminRoles(groupId);

Expand All @@ -132,6 +153,7 @@ public StudyRoleResponse getStudyRoles() {
* Set the roles for list of users
*/
public void setUserRoles(SetUserRoleRequest req) {
initialize();
int groupId = verifyStudyGroup(studyGroup);

Map<String, Integer> userIds = validateUsers(req.getUsers(), groupId);
Expand All @@ -154,14 +176,14 @@ public void setUserRoles(SetUserRoleRequest req) {

if (!rolesToAdd.isEmpty()) {
addRoles(userId, rolesToAdd, groupId, studyRoles);
log.info("Added roles for user {} in study group {}: {}", entry.getKey(), studyGroup,
String.join(", ", rolesToAdd));
log.info("[User admin] Operator {} added roles for user {} in study group {}: {}", adminEmail,
entry.getKey(), studyGroup, String.join(", ", rolesToAdd));
}

if (!rolesToRemove.isEmpty()) {
removeUserRoles(userId, rolesToRemove, groupId, studyRoles);
log.info("Removed roles for user {} in study group {}: {}", entry.getKey(), studyGroup,
String.join(", ", rolesToRemove));
log.info("[User admin] Operator {} removed roles for user {} in study group {}: {}", adminEmail,
entry.getKey(), studyGroup, String.join(", ", rolesToRemove));
}
}
}
Expand All @@ -170,6 +192,7 @@ public void setUserRoles(SetUserRoleRequest req) {
* Add and remove roles for list of users
*/
public void updateUserRoles(UpdateUserRoleRequest req) {
initialize();
int groupId = verifyStudyGroup(studyGroup);

if (CollectionUtils.isEmpty(req.getUsers())) {
Expand Down Expand Up @@ -210,16 +233,16 @@ public void updateUserRoles(UpdateUserRoleRequest req) {
if (hasAddRoles) {
for (var entry: userIds.entrySet()) {
addRoles(entry.getValue(), addRoles, groupId, studyRoles);
log.info("Added roles for user {} in study group {}: {}", entry.getKey(), studyGroup,
String.join(", ", addRoles));
log.info("[User admin] Operator {} added roles for user {} in study group {}: {}", adminEmail,
entry.getKey(), studyGroup, String.join(", ", addRoles));
}
}

if (hasRemoveRoles) {
for (var entry: userIds.entrySet()) {
removeUserRoles(entry.getValue(), removeRoles, groupId, studyRoles);
log.info("Removed roles for user {} in study group {}: {}", entry.getKey(), studyGroup,
String.join(", ", removeRoles));
log.info("[User admin] Operator {} removed roles for user {} in study group {}: {}", adminEmail,
entry.getKey(), studyGroup, String.join(", ", removeRoles));
}
}
}
Expand Down Expand Up @@ -294,6 +317,7 @@ protected static String validateEmailRequest(String email) {
}

public void addAndRemoveUsers(UserRequest req) {
initialize();
int groupId = verifyStudyGroup(studyGroup);
Map<String, RoleInfo> studyRoles = getAdminRoles(groupId);

Expand Down Expand Up @@ -339,19 +363,23 @@ protected void addUser(List<UserRequest.User> users, Map<String, UserDto> emailT
Map<String, RoleInfo> studyRoles) {
UserDao userDao = new UserDao();
for (var user: users) {
UserDto userDto = emailToUser.get(user.getEmail());
// email already verified
String email = user.getEmail();
UserDto userDto = emailToUser.get(email);
int userId = userDto.getId();
boolean hasUserSettings = false;
if (userId != 0) {
UserDao.update(userId, userDto);
hasUserSettings = UserSettings.getUserSettings(userDto.getEmailOrThrow()) != null;
hasUserSettings = UserSettings.getUserSettings(email) != null;
} else {
userId = userDao.create(userDto);
}
if (!hasUserSettings) {
UserSettings.createUserSettings(userId);
}
addRoles(userId, user.getRoles(), groupId, studyRoles);
log.info("[User admin] Operator {} added user {} to study group {} with roles {}", adminEmail, email,
studyGroup, String.join(", ", user.getRoles()));
}
}

Expand Down Expand Up @@ -405,6 +433,7 @@ protected UserDto verifyExistingUser(UserDto userDto, int groupId) {
}

public void updateUser(UpdateUserRequest req) {
initialize();
int groupId = validateOperatorAdmin();
List<UpdateUserRequest.User> users = req.getUsers();
if (CollectionUtils.isEmpty(users)) {
Expand All @@ -413,16 +442,21 @@ public void updateUser(UpdateUserRequest req) {

Map<Integer, UpdateUserRequest.User> usersById = new HashMap<>();
// pre-check to lessen likelihood of partial operation
for (var user: users) {
for (UpdateUserRequest.User user: users) {
// not a strict requirement in DB, but now enforcing
if (StringUtils.isBlank(user.getName())) {
throw new DSMBadRequestException("Invalid user name: blank");
}
usersById.put(verifyUserByEmail(user.getEmail(), groupId).getId(), user);
UserDto userDto = verifyUserByEmail(user.getEmail(), groupId);
user.setEmail(userDto.getEmailOrThrow());
usersById.put(userDto.getId(), user);
}

for (var entry: usersById.entrySet()) {
UserDao.update(entry.getKey(), entry.getValue().asUserDto());
UserDto userDto = entry.getValue().asUserDto();
UserDao.update(entry.getKey(), userDto);
log.info("[User admin] Operator {} updated information for user {} in study group {}",
adminEmail, userDto.getEmailOrThrow(), studyGroup);
}
}

Expand All @@ -435,6 +469,8 @@ protected void removeUser(List<UserDto> userDto) {
user.setIsActive(0);
UserDao.update(user.getId(), user);
UserSettings.deleteUserSettings(user.getId());
log.info("[User admin] Operator {} removed user {} from study group {}", adminEmail, user.getEmailOrThrow(),
studyGroup);
}
}

Expand All @@ -451,6 +487,7 @@ protected int validateOperatorAdmin() {
* @param req list of users, or all study users if NULL
*/
public UserRoleResponse getUserRoles(UserRoleRequest req) {
initialize();
int groupId = verifyStudyGroup(studyGroup);
Map<String, RoleInfo> studyRoles = getAdminRoles(groupId);

Expand Down Expand Up @@ -511,12 +548,6 @@ protected static List<UserRole> convertToUserRoles(Set<String> roles, Map<String
}

protected Map<String, RoleInfo> getAdminRoles(int studyGroupId) {
int adminId;
try {
adminId = Integer.parseInt(operatorId);
} catch (NumberFormatException e) {
throw new DSMBadRequestException("Invalid operator ID format: " + operatorId);
}
return verifyOperatorAdminRoles(adminId, studyGroupId);
}

Expand Down
3 changes: 2 additions & 1 deletion ...pis/dsm-core/src/test/java/org/broadinstitute/dsm/service/admin/UserAdminServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ public void testAddAndRemoveUser() {
int operatorId = setupAdmin("[email protected]", new ArrayList<>(rolesToId.values()), groupId);

String user = "[email protected]";
String userVariation = "[email protected]";
String userName = "test_user6";

UserAdminService service = new UserAdminService(Integer.toString(operatorId), TEST_GROUP);
Expand Down Expand Up @@ -545,7 +546,7 @@ public void testAddAndRemoveUser() {
// update user
String newUserName = "newName";
String phone = "555-1212";
UpdateUserRequest.User updateUser = new UpdateUserRequest.User(user, newUserName, phone);
UpdateUserRequest.User updateUser = new UpdateUserRequest.User(userVariation, newUserName, phone);
UpdateUserRequest updateReq = new UpdateUserRequest(List.of(updateUser));
try {
service.updateUser(updateReq);
Expand Down

0 comments on commit 590a8c0

Please sign in to comment.