SONAR-4475 Restore reverted commit by fixing errors catching when deleting a user

5 files changed, 41 insertions, 12 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java
index 3fdcc998ab8..409deb97f77 100644
--- a/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java
+++ b/sonar-server/src/main/java/org/sonar/server/permission/InternalPermissionService.java
@@ -24,6 +24,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.security.DefaultGroups;
+import org.sonar.api.utils.MessageException;
 import org.sonar.core.permission.ComponentPermissionFacade;
 import org.sonar.core.permission.Permission;
 import org.sonar.core.user.*;
@@ -71,6 +72,12 @@ public class InternalPermissionService implements ServerComponent {
     }
   }
 
+  public void checkAtLeastOneSysAdminExists(){
+    if (roleDao.countUserWithPermission(Permission.SYSTEM_ADMIN.key()) == 0){
+      throw MessageException.ofL10n("global_permissions.error.need_at_lest_one_admin", null);
+    }
+  }
+
   private void applyPermissionTemplate(String templateKey, String componentId) {
     permissionFacade.applyPermissionTemplate(templateKey, Long.parseLong(componentId));
   }
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb
index f2d5fd5d878..1105fa5292b 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/groups_controller.rb
@@ -23,7 +23,7 @@ class GroupsController < ApplicationController
   before_filter :admin_required
 
   def index
-    @groups = Group.find(:all, :order => 'name')
+    @groups = Group.all(:order => 'name')
     if params[:id]
       @group = Group.find(params[:id])
     else
@@ -72,7 +72,7 @@ class GroupsController < ApplicationController
   end
 
   def to_index(errors, id)
-    if !errors.empty?
+    unless errors.empty?
       flash[:error] = errors.full_messages.join("<br/>\n")
     end
 
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/users_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/users_controller.rb
index 97807f9a749..cf9104cc5e4 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/users_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/users_controller.rb
@@ -130,17 +130,20 @@ class UsersController < ApplicationController
   end
 
   def destroy
-    @user = User.find(params[:id])
-
-    if current_user.id==@user.id
-      flash[:error] = 'Please log in with another user in order to delete yourself.'
-
-    else
-      Api.users.deactivate(@user.login)
+    begin
+      user = User.find(params[:id])
+      Api.users.deactivate(user.login)
       flash[:notice] = 'User is deleted.'
+    rescue NativeException => exception
+      if exception.cause.java_kind_of? Java::OrgSonarServerExceptions::HttpException
+        error = exception.cause
+        flash[:error] = (error.getMessage ? error.getMessage : Api::Utils.message(error.l10nKey, :params => error.l10nParams.to_a))
+      else
+        flash[:error] = 'Error when deleting this user.'
+      end
     end
 
-    to_index(@user.errors, nil)
+    redirect_to(:action => 'index', :id => nil)
   end
 
   def select_group
@@ -158,7 +161,7 @@ class UsersController < ApplicationController
   end
 
   def to_index(errors, id)
-    if !errors.empty?
+    unless errors.empty?
       flash[:error] = errors.full_messages.join("<br/>\n")
     end
 
diff --git a/sonar-server/src/main/webapp/javascripts/application.js b/sonar-server/src/main/webapp/javascripts/application.js
index 773170b1a38..501fd589a24 100644
--- a/sonar-server/src/main/webapp/javascripts/application.js
+++ b/sonar-server/src/main/webapp/javascripts/application.js
@@ -320,8 +320,8 @@ function openModalWindow(url, options) {
                 $j('input[type=submit]', obj).removeAttr('disabled');
                 errorElt.show();
                 errorElt.html(xhr.responseText);
-                // otherwise replace modal window by the returned text
               } else {
+                // otherwise replace modal window by the returned text
                 $j("#modal").html(xhr.responseText);
               }
             }
diff --git a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java
index 6dd2f92fca6..bcff818c1cd 100644
--- a/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/permission/InternalPermissionServiceTest.java
@@ -30,6 +30,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.security.DefaultGroups;
+import org.sonar.api.utils.MessageException;
 import org.sonar.core.permission.ComponentPermissionFacade;
 import org.sonar.core.permission.Permission;
 import org.sonar.core.user.*;
@@ -40,6 +41,8 @@ import org.sonar.server.user.MockUserSession;
 
 import java.util.Map;
 
+import static org.fest.assertions.Assertions.assertThat;
+import static org.fest.assertions.Fail.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.argThat;
 import static org.mockito.Mockito.*;
@@ -191,6 +194,22 @@ public class InternalPermissionServiceTest {
     verify(permissionFacade).applyPermissionTemplate("my_template_key", 3L);
   }
 
+  @Test
+  public void should_check_at_least_one_sys_admin_exists() throws Exception {
+    // One admin exists -> all is ok
+    when(roleDao.countUserWithPermission(anyString())).thenReturn(1);
+    service.checkAtLeastOneSysAdminExists();
+
+    // No admin -> fail
+    try {
+      when(roleDao.countUserWithPermission(anyString())).thenReturn(0);
+      service.checkAtLeastOneSysAdminExists();
+      fail();
+    } catch (Exception e){
+      assertThat(e).isInstanceOf(MessageException.class);
+    }
+  }
+
   protected static class MatchesUserRole extends BaseMatcher<UserRoleDto> {
 
     private final UserRoleDto referenceDto;