SONAR-1722 improve the tab "Hierarchy" of inheritance of profiles

9 files changed, 104 insertions, 40 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java b/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java
index f17cb5194d9..a1ad040d321 100644
--- a/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java
+++ b/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java
@@ -26,6 +26,7 @@ import org.sonar.api.database.model.ResourceModel;
 import org.sonar.api.profiles.RulesProfile;
 import org.sonar.api.rules.ActiveRule;
 import org.sonar.api.rules.Rule;
+import org.sonar.api.utils.ValidationMessages;
 import org.sonar.jpa.dao.BaseDao;
 import org.sonar.jpa.dao.RulesDao;
 
@@ -87,13 +88,15 @@ public class ProfilesManager extends BaseDao {
 
   // Managing inheritance of profiles
 
-  public void changeParentProfile(Integer profileId, String parentName) {
+  public ValidationMessages changeParentProfile(Integer profileId, String parentName) {
+    ValidationMessages messages = ValidationMessages.create();
     RulesProfile profile = getSession().getEntity(RulesProfile.class, profileId);
     if (profile != null && !profile.getProvided()) {
       RulesProfile oldParent = getParentProfile(profile);
       RulesProfile newParent = getProfile(profile.getLanguage(), parentName);
       if (isCycle(profile, newParent)) {
-        return;
+        messages.addWarningText("Please do not select a child profile as parent.");
+        return messages;
       }
       // Deactivate all inherited rules
       if (oldParent != null) {
@@ -111,6 +114,7 @@ public class ProfilesManager extends BaseDao {
       getSession().saveWithoutFlush(profile);
       getSession().commit();
     }
+    return messages;
   }
 
   /**
diff --git a/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java b/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java
index 5aec6c8f921..60a792f5cc3 100644
--- a/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java
+++ b/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java
@@ -201,8 +201,8 @@ public final class JRubyFacade implements ServerComponent {
     getProfilesManager().deleteProfile((int) profileId);
   }
 
-  public void changeParentProfile(int profileId, String parentName) {
-    getProfilesManager().changeParentProfile(profileId, parentName);
+  public ValidationMessages changeParentProfile(int profileId, String parentName) {
+    return getProfilesManager().changeParentProfile(profileId, parentName);
   }
 
   public void ruleActivatedOrChanged(int parentProfileId, int activeRuleId) {
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb
index 3ce33ce9463..ea222e8aa56 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb
@@ -24,7 +24,7 @@ class ProfilesController < ApplicationController
   verify :method => :post, :only => ['create', 'delete', 'copy', 'set_as_default', 'restore', 'set_projects', 'rename', 'change_parent'], :redirect_to => { :action => 'index' }
 
   # the backup action is allow to non-admin users : see http://jira.codehaus.org/browse/SONAR-2039
-  before_filter :admin_required, :except => [ 'index', 'show', 'projects', 'permalinks', 'export', 'backup', 'inheritance' ]
+  before_filter :admin_required, :except => [ 'index', 'show', 'projects', 'permalinks', 'export', 'backup', 'hierarchy' ]
 
   #
   #
@@ -187,15 +187,14 @@ class ProfilesController < ApplicationController
 
   #
   #
-  # GET /profiles/inheritance?id=<profile id>
+  # GET /profiles/hierarchy?id=<profile id>
   #
   #
-  def inheritance
+  def hierarchy
     @profile = Profile.find(params[:id])
-    @child_profiles = Profile.find(:all,
-      :conditions => {:language => @profile.language, :parent_name => @profile.name},
-      :order => 'name')
-    @select_parent = [['', nil]] + Profile.find(:all).collect { |profile| [profile.name, profile.name] }.sort
+    
+    profiles=Profile.find(:all, :conditions => ['language=? and id<>? and (parent_name is null or parent_name<>?)', @profile.language, @profile.id, @profile.name], :order => 'name')
+    @select_parent = [['None', nil]] + profiles.collect{ |profile| [profile.name, profile.name] }
   end
 
 
@@ -208,11 +207,12 @@ class ProfilesController < ApplicationController
     id = params[:id].to_i
     parent_name = params[:parent_name]
     if parent_name.blank?
-      java_facade.changeParentProfile(id, nil)
+      messages = java_facade.changeParentProfile(id, nil)
     else
-      java_facade.changeParentProfile(id, parent_name)
+      messages = java_facade.changeParentProfile(id, parent_name)
     end
-    redirect_to :action => 'inheritance', :id => id
+    flash_validation_messages(messages)
+    redirect_to :action => 'hierarchy', :id => id
   end
 
 
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/profile.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/profile.rb
index 13b9559f964..36207a6dd34 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/profile.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/profile.rb
@@ -99,4 +99,38 @@ class Profile < ActiveRecord::Base
     end
     @active_hash_by_rule_id
   end
-end
+
+  def inherited?
+    parent_name.present?
+  end
+
+  def parent
+    @parent||=
+      begin
+        if parent_name.present?
+          Profile.find(:first, :conditions => ['language=? and name=?', language, parent_name])
+        else
+          nil
+        end
+      end
+  end
+
+  def ancestors
+    @ancestors ||=
+      begin
+        array=[]
+        if parent
+          array<<parent
+          array.concat(parent.ancestors)
+        end
+        array
+      end
+  end
+
+  def children
+    @children ||=
+      begin
+        Profile.find(:all, :conditions => ['language=? and parent_name=?', language, name], :order => 'name')
+      end
+  end
+end
\ No newline at end of file
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/_tabs.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/_tabs.html.erb
index fb9d6f7db4b..c1dad1849de 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/_tabs.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/_tabs.html.erb
@@ -16,7 +16,7 @@
     <a href="<%= url_for :controller => 'profiles', :action => 'permalinks', :id => @profile.id -%>" <%= "class='selected'" if selected_tab=='Permalinks' -%>>Permalinks</a>
   </li>
   <li>
-    <a href="<%= url_for :controller => 'profiles', :action => 'inheritance', :id => @profile.id -%>" <%= "class='selected'" if selected_tab=='Inheritance' -%>>Inheritance</a>
+    <a href="<%= url_for :controller => 'profiles', :action => 'hierarchy', :id => @profile.id -%>" <%= "class='selected'" if selected_tab=='Hierarchy' -%>>Hierarchy</a>
   </li>
   <% if new_tab %>
   <li>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/hierarchy.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/hierarchy.html.erb
new file mode 100644
index 00000000000..41c0df853a0
--- /dev/null
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/hierarchy.html.erb
@@ -0,0 +1,49 @@
+<h1 class="marginbottom10"><%= link_to 'Quality profiles', :controller => 'profiles', :action => 'index' -%> / <%= h @profile.language -%> / <%= h @profile.name %></h1>
+<%= render :partial => 'profiles/tabs', :locals => {:selected_tab=>'Hierarchy'} %>
+
+<div class="tabs-panel marginbottom10">
+
+  <table width="100%">
+    <tr>
+      <td align="center"  valign="top">
+        <div>
+            <% @profile.ancestors.reverse.each do |parent| %>
+              <a href="<%= url_for :action => 'hierarchy', :id => parent.id -%>"><%= parent.name -%></a> <span class="note">(<%= parent.active_rules.size -%> rules)</span><br/>
+              <%= image_tag 'blue-up.png' -%><br/>
+            <% end %>
+
+            <b><%= @profile.name -%></b> <span class="note">(<%= @profile.active_rules.size -%> rules)</span><br/>
+
+            <% if @profile.children.size>0 %>
+              <%= image_tag 'blue-up.png' -%><br/>
+              <% @profile.children.each_with_index do |child,index| %>
+                <%= ', ' if index>0 -%>
+                <a href="<%= url_for :action => 'hierarchy', :id => child.id -%>"><%= child.name -%></a> <span class="note">(<%= child.active_rules.size -%> rules)</span>
+              <% end %>
+              <br/><%= image_tag 'blue-up.png' -%><br/>
+              ...
+            <% end %>
+          </div>
+      </td>
+     <% if is_admin? %>
+       <td valign="top" width="300">
+         <div class="admin">
+         <% if @profile.provided? %>
+           <p>This profile can not be changed.</p>
+
+         <% else %>
+           <h3>Set parent:</h3>
+           <p>Inherit rules configuration from the profile:</p>
+           <% form_tag({:action => 'change_parent'}, {:method => 'post'}) do %>
+             <%= hidden_field_tag "id", @profile.id %>
+             <%= select_tag "parent_name", options_for_select(@select_parent, @profile.parent_name) %>
+             <%= submit_tag "Change", :id => 'submit_parent'%>
+           <% end %>
+         <% end %>
+         </div>
+       </td>
+     <% end %>
+    </tr>
+  </table>
+
+</div>
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/inheritance.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/inheritance.html.erb
deleted file mode 100644
index 0d34194ff36..00000000000
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/profiles/inheritance.html.erb
+++ /dev/null
@@ -1,21 +0,0 @@
-<h1 class="marginbottom10"><%= link_to 'Quality profiles', :controller => 'profiles', :action => 'index' -%> / <%= h @profile.language -%> / <%= h @profile.name %></h1>
-<%= render :partial => 'profiles/tabs', :locals => {:selected_tab=>'Inheritance'} %>
-
-<div class="tabs-panel marginbottom10 ">
-
-<% if !@profile.provided? %>
-  <% form_tag({:action => 'change_parent'}, {:method => 'post'}) do %>
-    <%= hidden_field_tag "id", @profile.id %>
-    Parent profile: <%= select_tag "parent_name", options_for_select(@select_parent, @profile.parent_name), :disabled => !is_admin? %>
-    <%= submit_tag "Change", :id => 'submit_parent', :disabled => !is_admin? %>
-  <% end %>
-
-  <% if @child_profiles.size > 0 %>
-    Inherited profiles:<br/>
-    <% @child_profiles.each do |child| %>
-    <a href="<%= url_for :controller => 'profiles', :action => 'inheritance', :id => child.id -%>"><%= child.name %></a><br/>
-    <% end %>
-  <% end %>
-<% end %>
-
-</div>
diff --git a/sonar-server/src/main/webapp/stylesheets/style.css b/sonar-server/src/main/webapp/stylesheets/style.css
index 98b72c1a42b..11e87e826c0 100644
--- a/sonar-server/src/main/webapp/stylesheets/style.css
+++ b/sonar-server/src/main/webapp/stylesheets/style.css
@@ -602,8 +602,6 @@ ul.operations li a {
   color: #555;
 }
 
-
-
 /* RESOURCE VIEWER */
 .resourceName h1 {
 	margin: 5px 0;
@@ -962,7 +960,6 @@ ul.operations li a {
   text-align: left;
   font-weight: bold;
   color: #333;
-  padding: 5px 10px;
 }
 
 .column {
diff --git a/sonar-server/src/test/java/org/sonar/server/configuration/InheritedProfilesTest.java b/sonar-server/src/test/java/org/sonar/server/configuration/InheritedProfilesTest.java
index 799fc7660e8..f1907ebcf53 100644
--- a/sonar-server/src/test/java/org/sonar/server/configuration/InheritedProfilesTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/configuration/InheritedProfilesTest.java
@@ -22,6 +22,7 @@ package org.sonar.server.configuration;
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.profiles.RulesProfile;
+import org.sonar.api.utils.ValidationMessages;
 import org.sonar.jpa.test.AbstractDbUnitTestCase;
 
 import static org.hamcrest.Matchers.is;