Skip to content

Commit

Permalink
DSM user admin error message fixes (#2645)
Browse files Browse the repository at this point in the history
* Clean up exception messsages returned in response

* Provide better error message for no user roles case
  • Loading branch information
denniscunningham authored Aug 4, 2023
1 parent ac44315 commit f46eed4
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ protected static String handleError(Throwable e, String operation, Response resp
if (e instanceof DSMBadRequestException) {
response.status(400);
log.info("DSMBadRequestException {}: {}", operation, e.toString());
return e.toString();
return e.getMessage();
} else if (e instanceof DsmInternalError) {
log.error("Error {}: {}", operation, e.toString());
response.status(500);
Expand All @@ -112,6 +112,6 @@ protected static String handleError(Throwable e, String operation, Response resp
// any other exception
log.error("Error {}: {}", operation, e.toString());
response.status(500);
return e.toString();
return e.getMessage();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ protected Map<String, Integer> validateUsers(List<String> users, int groupId) {
protected void validateRoles(List<String> roleNames, Set<String> validRoleNames) {
String msg = String.format("Invalid roles for study group %s: ", studyGroup);
if (CollectionUtils.isEmpty(roleNames)) {
throw new DSMBadRequestException(msg + "None provided");
throw new DSMBadRequestException(msg + "Users must have at least one role");
}
if (validRoleNames.containsAll(roleNames)) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ public void testValidateRoles() {
Assert.fail("UserAdminService.validateRoles should fail with no roles");
} catch (Exception e) {
Assert.assertTrue(e.getMessage().contains("Invalid roles for study group"));
Assert.assertTrue(e.getMessage().contains("None provided"));
Assert.assertTrue(e.getMessage().contains("must have at least one role"));
}

try {
Expand Down

0 comments on commit f46eed4

Please sign in to comment.