SONAR-8019 Order results with one with permissions

For /api/permissions/users
/api/permissions/groups
/api/permissions/template_users
/api/permissions/template_groups

server/sonar-db-dao/src/main/java/org/sonar/db/permission/PermissionQuery.java

@@ -152,7 +152,7 @@ public Builder setTemplate(@Nullable String template) {
       return this;
     }
 
-    public Builder setComponentUuid(@Nullable String componentUuid) {
+    public Builder setComponentUuid(String componentUuid) {
       this.componentUuid = componentUuid;
       return this;
     }

server/sonar-db-dao/src/main/java/org/sonar/db/permission/UserPermissionDao.java

@@ -56,7 +56,7 @@ public List<Integer> selectUserIdsByQuery(DbSession dbSession, PermissionQuery q
       // Pagination is done in Java because it's too complex to use SQL pagination in Oracle and MsSQL with the distinct
       .skip(query.getPageOffset())
       .limit(query.getPageSize())
-      .collect(MoreCollectors.toList());
+      .collect(MoreCollectors.toArrayList());
   }
 
   public int countUsersByQuery(DbSession dbSession, PermissionQuery query) {

server/sonar-db-dao/src/main/resources/org/sonar/db/permission/GroupPermissionMapper.xml

@@ -40,9 +40,10 @@
   </select>
 
   <select id="selectGroupNamesByQuery" parameterType="map" resultType="string">
-    select distinct sub.name, lower(sub.name), sub.groupId
+    select sub.name, lower(sub.name), sub.groupId
     <include refid="groupsByQuery" />
-    order by lower(sub.name), sub.name, sub.groupId
+    group by sub.name, lower(sub.name), sub.groupId
+    order by case when (count(sub.permission) > 0) then 1 else 2 end asc, lower(sub.name), sub.name, sub.groupId
   </select>
 
   <select id="countGroupsByQuery" parameterType="map" resultType="int">

server/sonar-db-dao/src/main/resources/org/sonar/db/permission/UserPermissionMapper.xml

@@ -18,12 +18,13 @@
 
   <select id="selectUserIdsByQuery" parameterType="map" resultType="int">
     select
-      distinct u.id, lower(u.name) as lowerName
-      <include refid="sqlQueryJoins" />
-      <where>
-        <include refid="sqlQueryFilters" />
-      </where>
-    order by lowerName asc
+      u.id, lower(u.name) as lowerName
+    <include refid="sqlQueryJoins" />
+    <where>
+      <include refid="sqlQueryFilters" />
+    </where>
+    group by u.id, lowerName
+    order by case when (count(ur.role) > 0) then 1 else 2 end asc, lowerName asc
   </select>
 
   <select id="countUsersByQuery" parameterType="map" resultType="int">

server/sonar-db-dao/src/main/resources/org/sonar/db/permission/template/PermissionTemplateMapper.xml

@@ -124,11 +124,10 @@
   </delete>
 
   <select id="selectUserLoginsByQueryAndTemplate" parameterType="map" resultType="string">
-    SELECT u.login FROM
-      (SELECT DISTINCT u.login AS login, u.name AS name
+    select u.login as login
       <include refid="userLoginsByQueryAndTemplate"/>
-      ) u
-    ORDER BY u.name
+    group by u.login, u.name
+    order by case when (count(ptu.permission_reference) > 0) then 1 else 2 end asc, u.name
   </select>
 
   <select id="countUserLoginsByQueryAndTemplate" parameterType="map" resultType="int">
@@ -157,9 +156,10 @@
   </sql>
 
   <select id="selectGroupNamesByQueryAndTemplate" parameterType="map" resultType="string">
-    SELECT DISTINCT groups.name, LOWER(groups.name), groups.group_id
+    select groups.name, lower(groups.name), groups.group_id
     <include refid="groupNamesByQueryAndTemplate" />
-    ORDER BY LOWER(groups.name), groups.name, groups.group_id
+    group by groups.name, lower(groups.name), groups.group_id
+    order by case when (count(groups.permission) > 0) then 1 else 2 end asc, lower(groups.name), groups.name, groups.group_id
   </select>
 
   <select id="countGroupNamesByQueryAndTemplate" parameterType="map" resultType="int">

server/sonar-db-dao/src/test/java/org/sonar/db/permission/GroupPermissionDaoTest.java

@@ -126,15 +126,16 @@ public void group_count_by_permission_and_component_id_on_public_projects() {
   }
 
   @Test
-  public void selectGroupNamesByQuery_is_ordered_by_group_names() {
+  public void selectGroupNamesByQuery_is_ordered_by_permissions_then_by_group_names() {
     OrganizationDto organizationDto = db.organizations().insert();
     GroupDto group2 = db.users().insertGroup(organizationDto, "Group-2");
     GroupDto group3 = db.users().insertGroup(organizationDto, "Group-3");
     GroupDto group1 = db.users().insertGroup(organizationDto, "Group-1");
     db.users().insertPermissionOnAnyone(organizationDto, SCAN);
+    db.users().insertPermissionOnGroup(group3, SCAN);
 
     assertThat(underTest.selectGroupNamesByQuery(dbSession, newQuery().setOrganizationUuid(organizationDto.getUuid()).build()))
-      .containsExactly(ANYONE, group1.getName(), group2.getName(), group3.getName());
+      .containsExactly(ANYONE, group3.getName(), group1.getName(), group2.getName());
   }
 
   @Test

server/sonar-db-dao/src/test/java/org/sonar/db/permission/UserPermissionDaoTest.java

@@ -43,6 +43,7 @@
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.tuple;
+import static org.sonar.api.web.UserRole.ADMIN;
 import static org.sonar.api.web.UserRole.CODEVIEWER;
 import static org.sonar.api.web.UserRole.ISSUE_ADMIN;
 import static org.sonar.api.web.UserRole.USER;
@@ -155,6 +156,41 @@ public void select_project_permissions() {
     expectPermissions(query, emptyList());
   }
 
+  @Test
+  public void selectUserIdsByQuery_is_ordering_by_users_having_permissions_first_then_by_name_lowercase() {
+    OrganizationDto organization = db.organizations().insert();
+    UserDto user1 = insertUser(u -> u.setLogin("login1").setName("Z").setEmail("email1@email.com"), organization);
+    UserDto user2 = insertUser(u -> u.setLogin("login2").setName("A").setEmail("email2@email.com"), organization);
+    UserDto user3 = insertUser(u -> u.setLogin("login3").setName("Z").setEmail("zanother3@another.com"), organization);
+    UserDto user4 = insertUser(u -> u.setLogin("login4").setName("A").setEmail("zanother3@another.com"), organization);
+    addGlobalPermission(organization, SYSTEM_ADMIN, user1);
+    ComponentDto project1 = db.components().insertPrivateProject(organization);
+    addProjectPermission(organization, USER, user2, project1);
+
+    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).build();
+
+    assertThat(underTest.selectUserIdsByQuery(dbSession, query))
+      .containsExactly(user2.getId(), user1.getId(), user4.getId(), user3.getId());
+  }
+
+  @Test
+  public void selectUserIdsByQuery_is_not_ordering_by_number_of_permissions() {
+    OrganizationDto organization = db.organizations().insert();
+    UserDto user1 = insertUser(u -> u.setLogin("login1").setName("Z").setEmail("email1@email.com"), organization);
+    UserDto user2 = insertUser(u -> u.setLogin("login2").setName("A").setEmail("email2@email.com"), organization);
+    addGlobalPermission(organization, SYSTEM_ADMIN, user1);
+    ComponentDto project1 = db.components().insertPrivateProject(organization);
+    addProjectPermission(organization, USER, user2, project1);
+    addProjectPermission(organization, USER, user1, project1);
+    addProjectPermission(organization, ADMIN, user1, project1);
+
+    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).build();
+
+    // Even if user1 has 3 permissions, the name is used to order
+    assertThat(underTest.selectUserIdsByQuery(dbSession, query))
+      .containsExactly(user2.getId(), user1.getId());
+  }
+
   @Test
   public void countUsersByProjectPermission() {
     OrganizationDto organization = db.organizations().insert();

server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/GroupWithPermissionTemplateDaoTest.java

@@ -25,6 +25,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.api.utils.System2;
+import org.sonar.api.web.UserRole;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.organization.OrganizationDto;
@@ -87,17 +88,19 @@ public void select_group_names_by_query_and_template() {
   }
 
   @Test
-  public void select_group_names_by_query_and_template_is_ordered_by_group_names() {
+  public void selectGroupNamesByQueryAndTemplate_is_ordering_results_by_groups_with_permission_then_by_name() {
     OrganizationDto organization = db.organizations().insert();
-    GroupDto group2 = db.users().insertGroup(organization, "Group-2");
-    db.users().insertGroup(organization, "Group-3");
-    db.users().insertGroup(organization, "Group-1");
-
     PermissionTemplateDto template = permissionTemplateDbTester.insertTemplate(organization);
-    permissionTemplateDbTester.addGroupToTemplate(template.getId(), group2.getId(), USER);
 
-    assertThat(selectGroupNamesByQueryAndTemplate(builder(), organization, template))
-      .containsExactly("Anyone", "Group-1", "Group-2", "Group-3");
+    GroupDto group1 = db.users().insertGroup(organization, "A");
+    GroupDto group2 = db.users().insertGroup(organization, "B");
+    GroupDto group3 = db.users().insertGroup(organization, "C");
+
+    permissionTemplateDbTester.addGroupToTemplate(template, group3, UserRole.USER);
+
+    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).build();
+    assertThat(underTest.selectGroupNamesByQueryAndTemplate(db.getSession(), query, template.getId()))
+      .containsExactly("Anyone", group3.getName(), group1.getName(), group2.getName());
   }
 
   @Test

server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/PermissionTemplateDaoTest.java

@@ -31,6 +31,7 @@
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.organization.OrganizationDto;
+import org.sonar.db.permission.PermissionQuery;
 import org.sonar.db.user.GroupDto;
 import org.sonar.db.user.UserDto;
 

server/sonar-db-dao/src/test/java/org/sonar/db/permission/template/UserWithPermissionTemplateDaoTest.java

@@ -23,9 +23,11 @@
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.api.utils.System2;
+import org.sonar.api.web.UserRole;
 import org.sonar.db.DbSession;
 import org.sonar.db.DbTester;
 import org.sonar.db.organization.OrganizationDto;
+import org.sonar.db.permission.PermissionQuery;
 import org.sonar.db.user.UserDto;
 
 import static java.util.Arrays.asList;
@@ -143,19 +145,18 @@ dbSession, builder().setOrganizationUuid(organization.getUuid()).withAtLeastOneP
   }
 
   @Test
-  public void should_be_sorted_by_user_name() {
+  public void selectUserLoginsByQueryAndTemplate_is_ordering_result_by_users_with_permissions_then_by_name() {
     OrganizationDto organization = db.organizations().insert();
-    UserDto user1 = db.users().insertUser(u -> u.setName("User3"));
-    UserDto user2 = db.users().insertUser(u -> u.setName("User1"));
-    UserDto user3 = db.users().insertUser(u -> u.setName("User2"));
+    PermissionTemplateDto template = db.permissionTemplates().insertTemplate(organization);
+    UserDto user1 = db.users().insertUser(u -> u.setName("A"));
+    UserDto user2 = db.users().insertUser(u -> u.setName("B"));
+    UserDto user3 = db.users().insertUser(u -> u.setName("C"));
     db.organizations().addMember(organization, user1, user2, user3);
-    PermissionTemplateDto permissionTemplate = db.permissionTemplates().insertTemplate();
-    db.permissionTemplates().addUserToTemplate(permissionTemplate, user1, USER);
-    db.permissionTemplates().addUserToTemplate(permissionTemplate, user2, USER);
+    db.permissionTemplates().addUserToTemplate(template.getId(), user3.getId(), UserRole.USER);
 
-    assertThat(underTest.selectUserLoginsByQueryAndTemplate(dbSession,
-      builder().setOrganizationUuid(organization.getUuid()).build(), permissionTemplate.getId()))
-        .containsExactly(user2.getLogin(), user3.getLogin(), user1.getLogin());
+    PermissionQuery query = PermissionQuery.builder().setOrganizationUuid(organization.getUuid()).build();
+    assertThat(underTest.selectUserLoginsByQueryAndTemplate(db.getSession(), query, template.getId()))
+      .containsExactly(user3.getLogin(), user1.getLogin(), user2.getLogin());
   }
 
   @Test

server/sonar-server/src/main/java/org/sonar/server/permission/ws/GroupsAction.java

@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.Optional;
 import org.sonar.api.security.DefaultGroups;
+import org.sonar.api.server.ws.Change;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
@@ -81,11 +82,13 @@ public void define(WebService.NewController context) {
         "<li>'Administer' rights on the specified project</li>" +
         "</ul>")
       .addPagingParams(DEFAULT_PAGE_SIZE, RESULTS_MAX_SIZE)
+      .setChangelog(
+        new Change("7.4", "The response list is returning all groups even those without permissions, the groups with permission are at the top of the list."))
       .setResponseExample(Resources.getResource(getClass(), "groups-example.json"))
       .setHandler(this);
 
     action.createSearchQuery("sonar", "names")
-      .setDescription("Limit search to group names that contain the supplied string. When this parameter is not set, only groups having at least one permission are returned.")
+      .setDescription("Limit search to group names that contain the supplied string.")
       .setMinimumLength(SEARCH_QUERY_MIN_LENGTH);
 
     createOrganizationParameter(action).setSince("6.2");
@@ -122,9 +125,7 @@ private static PermissionQuery buildPermissionQuery(Request request, Organizatio
     if (project.isPresent()) {
       permissionQuery.setComponentUuid(project.get().getUuid());
     }
-    if (textQuery == null) {
-      permissionQuery.withAtLeastOnePermission();
-    }
+
     return permissionQuery.build();
   }
 

server/sonar-server/src/main/java/org/sonar/server/permission/ws/RemoveUserAction.java

@@ -33,7 +33,7 @@
 import org.sonar.server.permission.UserPermissionChange;
 import org.sonar.server.user.UserSession;
 
-import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
 import static org.sonar.server.permission.PermissionPrivilegeChecker.checkProjectAdmin;
 import static org.sonar.server.permission.ws.PermissionsWsParametersBuilder.createOrganizationParameter;
 import static org.sonar.server.permission.ws.PermissionsWsParametersBuilder.createPermissionParameter;
@@ -94,7 +94,7 @@ public void handle(Request request, Response response) throws Exception {
         request.mandatoryParam(PARAM_PERMISSION),
         projectId.orElse(null),
         user);
-      permissionUpdater.apply(dbSession, asList(change));
+      permissionUpdater.apply(dbSession, singletonList(change));
       response.noContent();
     }
   }

server/sonar-server/src/main/java/org/sonar/server/permission/ws/UsersAction.java

@@ -25,6 +25,7 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.Collectors;
+import org.sonar.api.server.ws.Change;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
@@ -85,14 +86,15 @@ public void define(WebService.NewController context) {
         "<li>'Administer' rights on the specified project</li>" +
         "</ul>")
       .addPagingParams(DEFAULT_PAGE_SIZE, RESULTS_MAX_SIZE)
+      .setChangelog(
+        new Change("7.4", "The response list is returning all users even those without permissions, the users with permission are at the top of the list."))
       .setInternal(true)
       .setResponseExample(getClass().getResource("users-example.json"))
       .setHandler(this);
 
     action.createParam(Param.TEXT_QUERY)
       .setMinimumLength(SEARCH_QUERY_MIN_LENGTH)
-      .setDescription("Limit search to user names that contain the supplied string. <br/>" +
-        "When this parameter is not set, only users having at least one permission are returned.")
+      .setDescription("Limit search to user names that contain the supplied string. <br/>")
       .setExampleValue("eri");
 
     createOrganizationParameter(action).setSince("6.2");
@@ -134,9 +136,7 @@ private static PermissionQuery buildPermissionQuery(Request request, Organizatio
         validateGlobalPermission(permission);
       }
     }
-    if (textQuery == null) {
-      permissionQuery.withAtLeastOnePermission();
-    }
+
     return permissionQuery.build();
   }
 

server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/TemplateGroupsAction.java

@@ -113,9 +113,6 @@ private static PermissionQuery buildPermissionQuery(Request request, PermissionT
       .setPageIndex(request.mandatoryParamAsInt(PAGE))
       .setPageSize(request.mandatoryParamAsInt(PAGE_SIZE))
       .setSearchQuery(textQuery);
-    if (textQuery == null) {
-      permissionQuery.withAtLeastOnePermission();
-    }
     return permissionQuery.build();
   }
 

server/sonar-server/src/main/java/org/sonar/server/permission/ws/template/TemplateUsersAction.java

@@ -121,9 +121,6 @@ private static PermissionQuery buildQuery(Request wsRequest, PermissionTemplateD
       .setPageIndex(wsRequest.mandatoryParamAsInt(PAGE))
       .setPageSize(wsRequest.mandatoryParamAsInt(PAGE_SIZE))
       .setSearchQuery(textQuery);
-    if (textQuery == null) {
-      query.withAtLeastOnePermission();
-    }
     return query.build();
   }
 

server/sonar-server/src/test/java/org/sonar/server/permission/ws/UsersActionTest.java

@@ -119,29 +119,6 @@ public void search_for_users_with_permission_on_project() {
       .doesNotContain(withoutPermission.getLogin());
   }
 
-  @Test
-  public void search_only_for_users_with_permission_when_no_search_query() {
-    // User have permission on project
-    ComponentDto project = db.components().insertPrivateProject();
-    UserDto user = db.users().insertUser();
-    db.organizations().addMember(db.getDefaultOrganization(), user);
-    db.users().insertProjectPermissionOnUser(user, ISSUE_ADMIN, project);
-
-    // User has no permission
-    UserDto withoutPermission = db.users().insertUser();
-    db.organizations().addMember(db.getDefaultOrganization(), withoutPermission);
-
-    loginAsAdmin(db.getDefaultOrganization());
-    String result = newRequest()
-      .setParam(PARAM_PROJECT_ID, project.uuid())
-      .execute()
-      .getInput();
-
-    assertThat(result)
-      .contains(user.getLogin())
-      .doesNotContain(withoutPermission.getLogin());
-  }
-
   @Test
   public void search_also_for_users_without_permission_when_filtering_name() {
     // User with permission on project

server/sonar-server/src/test/java/org/sonar/server/permission/ws/template/TemplateGroupsActionTest.java

@@ -130,7 +130,7 @@ public void return_all_permissions_of_matching_groups() {
       .setParam(PARAM_TEMPLATE_ID, template.getUuid())
       .executeProtobuf(WsGroupsResponse.class);
 
-    assertThat(response.getGroupsList()).extracting("name").containsExactly("Anyone", "group-1-name", "group-2-name");
+    assertThat(response.getGroupsList()).extracting("name").containsExactly("Anyone", "group-1-name", "group-2-name", "group-3-name");
     assertThat(response.getGroups(0).getPermissionsList()).containsOnly("user", "issueadmin");
     assertThat(response.getGroups(1).getPermissionsList()).containsOnly("codeviewer", "admin");
     assertThat(response.getGroups(2).getPermissionsList()).containsOnly("user", "admin");
@@ -188,7 +188,7 @@ public void search_by_template_name() {
       .setParam(PARAM_TEMPLATE_NAME, template.getName())
       .executeProtobuf(WsGroupsResponse.class);
 
-    assertThat(response.getGroupsList()).extracting("name").containsExactly("Anyone", "group-1-name", "group-2-name");
+    assertThat(response.getGroupsList()).extracting("name").containsExactly("Anyone", "group-1-name", "group-2-name", "group-3-name");
   }
 
   @Test