Skip to content

Commit

Permalink
CLDR-14788 Refactor to avoid front-end permission assumptions
Browse files Browse the repository at this point in the history
-Get userCanListUsers and userCanUseVettingParticipation from back end

-Rename canDoList to canListUsers for clarity

-Remove unused method UserRegistry.getPermissions returning Map

-Update unit tests
  • Loading branch information
btangmu committed Jul 29, 2024
1 parent 2881add commit 2c99749
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 43 deletions.
2 changes: 1 addition & 1 deletion tools/cldr-apps/js/src/esm/cldrAccount.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ function getTableEnd(json) {
}

function canListMultipleUsers() {
return !!cldrStatus.getPermissions()?.userIsManager;
return !!cldrStatus.getPermissions()?.userCanListUsers;
}

function numberOfUsersShown(number) {
Expand Down
3 changes: 2 additions & 1 deletion tools/cldr-apps/js/src/views/MainMenu.vue
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ export default {
const perm = cldrStatus.getPermissions();
this.accountLocked = perm && perm.userIsLocked;
this.canImportOldVotes = perm && perm.userCanImportOldVotes;
this.canListUsers = this.canMonitorVetting = !!perm?.userIsManager;
this.canListUsers = !!perm?.userCanListUsers;
this.canMonitorVetting = !!perm?.userCanUseVettingParticipation;
this.canMonitorForum = perm && perm.userCanMonitorForum;
// this.canSeeStatistics will be false until there is a new implementation
this.canUseVettingSummary = perm && perm.userCanUseVettingSummary;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,7 @@ private void processRequest(
mySession.userDidAction();
SurveyJSONWrapper r = newJSONStatus(request, sm);
r.put("what", what);
if (UserRegistry.userIsManagerOrStronger(mySession.user)) {
if (UserRegistry.userCanUseVettingParticipation(mySession.user)) {
String org = mySession.user.org;
if (UserRegistry.userCreateOtherOrgs(mySession.user)) {
org = null; // all
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public UserList(
}
canShowLocked = UserRegistry.userIsExactlyManager(me) || UserRegistry.userIsTC(me);
showLocked = canShowLocked && ctx.prefBool(PREF_SHOWLOCKED);
isValid = isJustMe || UserRegistry.userCanDoList(me);
isValid = isJustMe || UserRegistry.userCanListUsers(me);
}

public void getJson(SurveyJSONWrapper r) throws JSONException, SurveyException, IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import java.sql.SQLException;
import java.sql.Statement;
import java.util.*;
import javax.json.bind.annotation.JsonbProperty;
import org.apache.commons.codec.digest.DigestUtils;
import org.eclipse.microprofile.openapi.annotations.media.Schema;
import org.json.JSONException;
Expand Down Expand Up @@ -504,37 +503,16 @@ JSONObject getPermissionsJson() throws JSONException {
.put("userCanImportOldVotes", canImportOldVotes())
.put("userCanUseVettingSummary", userCanUseVettingSummary(this))
.put("userCanCreateSummarySnapshot", userCanCreateSummarySnapshot(this))
.put("userCanListUsers", userCanListUsers(this))
.put("userCanMonitorForum", userCanMonitorForum(this))
.put("userCanUseVettingParticipation", userCanUseVettingParticipation(this))
.put("userIsAdmin", userIsAdmin(this))
.put("userIsManager", getLevel().canManageSomeUsers())
.put("userIsTC", userIsTC(this))
.put("userIsVetter", userIsVetter(this) && !userIsTC(this))
.put("userIsLocked", userIsLocked(this));
}

/**
* this property is called permissionsJson for compatiblity
*
* @return
*/
@JsonbProperty("permissionsJson")
@Schema(description = "array of permissions for this user")
public Map<String, Boolean> getPermissions() {
Map<String, Boolean> m = new HashMap<>();

m.put("userCanImportOldVotes", canImportOldVotes());
m.put("userCanUseVettingSummary", userCanUseVettingSummary(this));
m.put("userCanCreateSummarySnapshot", userCanCreateSummarySnapshot(this));
m.put("userCanMonitorForum", userCanMonitorForum(this));
m.put("userIsAdmin", userIsAdmin(this));
m.put("userIsTC", userIsTC(this));
m.put("userIsManager", getLevel().canManageSomeUsers());
m.put("userIsVetter", userIsVetter(this) && !userIsTC(this));
m.put("userIsLocked", userIsLocked(this));

return m;
}

public void setPassword(String randomPass) {
this.password = randomPass;
}
Expand Down Expand Up @@ -1812,8 +1790,12 @@ && userCanModifyUser(managerUser, targetId, targetLevel)
&& targetLevel > managerUser.userlevel;
}

static boolean userCanDoList(User managerUser) {
return (managerUser != null) && managerUser.getLevel().canDoList();
static boolean userCanListUsers(User managerUser) {
return (managerUser != null) && managerUser.getLevel().canListUsers();
}

static boolean userCanUseVettingParticipation(User managerUser) {
return (managerUser != null) && managerUser.getLevel().canUseVettingParticipation();
}

public static boolean userCanCreateUsers(User u) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ void TestCompatUserLevelDataDriven(
@CsvSource({
// List of all operations of the form UserRegistry.<operation>(null)
// org, null, operation, expStr, otherOrg, otherLevel
"adlam, null, userCanDoList, false, wod_nko, vetter",
"adlam, null, userCanListUsers, false, wod_nko, vetter",
"adlam, null, userCanUseVettingParticipation, false, wod_nko, vetter",
"adlam, null, userCanCreateUsers, false, wod_nko, vetter",
"adlam, null, userCanEmailUsers, false, wod_nko, vetter",
"adlam, null, userCanModifyUsers, false, wod_nko, vetter",
Expand Down Expand Up @@ -123,8 +124,11 @@ private void testCompatAction(
case "canImportOldVotesSUBMISSION":
assertEquals(expected, u.canImportOldVotes(CheckCLDR.Phase.SUBMISSION), onFail);
break;
case "userCanDoList":
assertEquals(expected, UserRegistry.userCanDoList(u), onFail);
case "userCanListUsers":
assertEquals(expected, UserRegistry.userCanListUsers(u), onFail);
break;
case "userCanUseVettingParticipation":
assertEquals(expected, UserRegistry.userCanUseVettingParticipation(u), onFail);
break;
case "userCanCreateUsers":
assertEquals(expected, UserRegistry.userCanCreateUsers(u), onFail);
Expand Down Expand Up @@ -259,8 +263,11 @@ void TestVoteResolverLevel(
case "canImportOldVotesSUBMISSION":
assertEquals(expected, l.canImportOldVotes(CheckCLDR.Phase.SUBMISSION), onFail);
break;
case "userCanDoList":
assertEquals(expected, l.canDoList(), onFail);
case "canListUsers":
assertEquals(expected, l.canListUsers(), onFail);
break;
case "userCanUseVettingParticipation":
assertEquals(expected, l.canUseVettingParticipation(), onFail);
break;
case "userCanCreateUsers":
assertEquals(expected, l.canCreateUsers(), onFail);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,20 @@ adlam, vetter, canImportOldVotesSUBMISSION, true, ,
adlam, guest, canImportOldVotesSUBMISSION, false, ,
adlam, locked, canImportOldVotesSUBMISSION, false, ,
adlam, anonymous, canImportOldVotesSUBMISSION, false, ,
adlam, admin,userCanDoList, true, ,
adlam, tc,userCanDoList, true, ,
adlam, manager,userCanDoList, true, ,
adlam, vetter,userCanDoList, false, ,
adlam, guest,userCanDoList, false, ,
adlam, locked,userCanDoList, false, ,
adlam, anonymous,userCanDoList, false, ,
adlam, admin, userCanListUsers, true, ,
adlam, tc, userCanListUsers, true, ,
adlam, manager, userCanListUsers, true, ,
adlam, vetter, userCanListUsers, false, ,
adlam, guest, userCanListUsers, false, ,
adlam, locked, userCanListUsers, false, ,
adlam, anonymous, userCanListUsers, false, ,
adlam, admin, userCanUseVettingParticipation, true, ,
adlam, tc, userCanUseVettingParticipation, true, ,
adlam, manager, userCanUseVettingParticipation, true, ,
adlam, vetter, userCanUseVettingParticipation, false, ,
adlam, guest, userCanUseVettingParticipation, false, ,
adlam, locked, userCanUseVettingParticipation, false, ,
adlam, anonymous, userCanUseVettingParticipation, false, ,
adlam, admin,userCanCreateUsers, true, ,
adlam, tc,userCanCreateUsers, true, ,
adlam, manager,userCanCreateUsers, true, ,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ public boolean canImportOldVotes(CheckCLDR.Phase inPhase) {
return isVetter() && (inPhase == Phase.SUBMISSION);
}

public boolean canDoList() {
public boolean canListUsers() {
return isManagerOrStronger();
}

Expand Down Expand Up @@ -434,6 +434,10 @@ public boolean canGetEmailList() {
public boolean canDeleteUsers() {
return isAdmin();
}

public boolean canUseVettingParticipation() {
return isManagerOrStronger();
}
}

/**
Expand Down

0 comments on commit 2c99749

Please sign in to comment.