-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Regression test for Impersonating Troubleshooter #1697
Regression test for Impersonating Troubleshooter #1697
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Just a couple optional suggestions.
@@ -40,11 +42,18 @@ public class AppAdminRoleTest extends BaseWebDriverTest | |||
{ | |||
private static final String APP_ADMIN = "[email protected]"; | |||
private static final String USER = "[email protected]"; | |||
private static final String ADMIN_GROUP = "Custom Admin Group"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SITE_ADMIN_GROUP
and "Custom Site Admin Group"
for clarity?
public static final String DEVELOPER_ROLE = "Platform Developer"; | ||
public static final String IMP_TROUBLESHOOTER_ROLE = "Impersonating Troubleshooter"; | ||
|
||
public static String toRole(final String name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should callers use the above constants to specify common roles instead of hard-coded strings?
Just FYI: I have a local change that disables the privileged roles on the permissions page for App Admins, which I think is a better experience. This test will need to be updated when (or before) that's merged. |
Rationale
The new
ImpersonatingTroubleshooterRole
needs testing.PermissionsEditor
automatically saves after several operations, which makes it difficult to test error conditions. It isn't widely used however (it duplicates a lot of functionality from the more populareUIPermissionsHelper
).Story
Related Pull Requests
Changes
PermissionsEditor
test pagePermissionsHelper.getRoles
static