From: David Gageot <david@gageot.net>
Date: Thu, 24 May 2012 08:13:12 +0000 (+0200)
Subject: SONAR-3460 fix default dashboards
X-Git-Tag: 3.1~83
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=8b4e95d423875d4317a4777f4c972c571fd2e291;p=sonarqube.git

SONAR-3460 fix default dashboards

There is one use which was not correctly covered :
 + A user has tuned his list Filters but not his list of project
   dashboards
 + During migration to Sonar 3.1, his dedicated list of global
   dashboards is created (in the active_dashboards table) but this list
   doesn't contain the list of project dashboard
 + So as soon as this user creates its own project dashboard, all the
   other project dashboards are no more displayed for this user
---

diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/admin_dashboards_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/admin_dashboards_controller.rb
index 5f5abfd24b2..18f7908ab36 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/admin_dashboards_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/admin_dashboards_controller.rb
@@ -21,34 +21,32 @@ class AdminDashboardsController < ApplicationController
 
   SECTION=Navigation::SECTION_CONFIGURATION
 
-  verify :method => :post, :only => [:up, :down, :remove, :add, :delete], :redirect_to => {:action => :index}
+  verify :method => :post, :only => [:up, :down, :remove, :add], :redirect_to => {:action => :index}
   before_filter :admin_required
+  before_filter :load_default_dashboards
 
   def index
-    load_default_dashboards
-    ids=@actives.map { |a| a.dashboard_id }
-    @shared_dashboards=Dashboard.find(:all, :conditions => { :shared => true }).sort { |a, b| a.name.downcase<=>b.name.downcase }
+    ids=@actives.map(&:dashboard_id)
+    @shared_dashboards=Dashboard.find(:all, :conditions => {:shared => true}).sort { |a, b| a.name.downcase<=>b.name.downcase }
     @shared_dashboards.reject! { |s| ids.include?(s.id) }
   end
 
   def down
     position(+1)
-
     redirect_to :action => 'index'
   end
 
   def up
     position(-1)
-
     redirect_to :action => 'index'
   end
 
   def add
-    load_default_dashboards
+    dashboard=Dashboard.find(params[:id])
+    if dashboard and dashboard.shared?
+      last_index = @actives.max_by(&:order_index).order_index
 
-    dashboard=Dashboard.find(:first, :conditions => { :shared => true, :id => params[:id].to_i })
-    if dashboard
-      ActiveDashboard.create(:dashboard => dashboard, :user => nil, :order_index => @actives.max_by(&:order_index).order_index+1)
+      ActiveDashboard.create(:dashboard => dashboard, :order_index => last_index+1)
       flash[:notice]='Default dashboard added.'
     end
 
@@ -56,12 +54,10 @@ class AdminDashboardsController < ApplicationController
   end
 
   def remove
-    load_default_dashboards
-
     if @actives.size<=1
       flash[:error]='At least one dashboard must be defined as default.'
     else
-      active=@actives.find { |af| af.id==params[:id].to_i }
+      active=@actives.find { |a| a.id==params[:id].to_i }
       if active
         active.destroy
         flash[:notice]='Dashboard removed from default dashboards.'
@@ -71,20 +67,6 @@ class AdminDashboardsController < ApplicationController
     redirect_to :action => 'index'
   end
 
-  def delete
-    dashboard=Dashboard.find(params[:id])
-    bad_request('Bad dashboard') unless dashboard
-    bad_request('This dashboard can not be deleted') unless dashboard.provided_programmatically?
-
-    if dashboard.destroy
-      flash[:notice]="Dashboard #{dashboard.name(true)} deleted."
-    else
-      flash[:error]="Can't be deleted as long as it's used as default dashboard."
-    end
-
-    redirect_to :action => 'index'
-  end
-
   private
 
   def load_default_dashboards
@@ -92,16 +74,14 @@ class AdminDashboardsController < ApplicationController
   end
 
   def position(offset)
-    load_default_dashboards
-
-    to_move = @actives.find { |a| a.id == params[:id].to_i}
+    to_move = @actives.find { |a| a.id == params[:id].to_i }
     if to_move
       dashboards_same_type=@actives.select { |a| (a.global? == to_move.global?) }.sort_by(&:order_index)
 
       index = dashboards_same_type.index(to_move)
       dashboards_same_type[index], dashboards_same_type[index + offset] = dashboards_same_type[index + offset], dashboards_same_type[index]
 
-      dashboards_same_type.each_with_index do |a,i|
+      dashboards_same_type.each_with_index do |a, i|
         a.order_index=i+1
         a.save
       end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/dashboards_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/dashboards_controller.rb
index bf45f8149de..2706f3125fc 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/dashboards_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/dashboards_controller.rb
@@ -28,15 +28,14 @@ class DashboardsController < ApplicationController
     @global = !params[:resource]
 
     @actives=ActiveDashboard.user_dashboards(current_user, @global)
-    @shared_dashboards=Dashboard.find(:all, :conditions => ['(user_id<>? OR user_id IS NULL) AND shared=?', current_user.id, true], :order => 'name ASC')
-    active_dashboard_ids=@actives.map(&:dashboard_id)
-    @shared_dashboards.reject! { |d| active_dashboard_ids.include?(d.id) }
-    @shared_dashboards.reject! { |a| a.global != @global}
+    @shared_dashboards=Dashboard.find(:all, :conditions => ['(user_id<>? OR user_id IS NULL) AND shared=? AND is_global=?', current_user.id, true, @global]).sort { |a, b| a.name.downcase<=>b.name.downcase }
+    active_ids=@actives.map(&:dashboard_id)
+    @shared_dashboards.reject! { |d| active_ids.include?(d.id) }
 
     if params[:resource]
       @resource=Project.by_key(params[:resource])
       if @resource.nil?
-      # TODO display error page
+        # TODO display error page
         redirect_to home_path
         return false
       end
@@ -54,14 +53,17 @@ class DashboardsController < ApplicationController
     active_dashboard = current_user.active_dashboards.to_a.find { |ad| ad.name==@dashboard.name }
     if active_dashboard
       flash[:error]=Api::Utils.message('dashboard.error_create_existing_name')
+
       redirect_to :action => 'index', :resource => params[:resource]
     elsif @dashboard.save
-      add_default_dashboards_if_first_user_dashboard
-      last_active_dashboard=current_user.active_dashboards.max_by(&:order_index)
-      current_user.active_dashboards.create(:dashboard => @dashboard, :user_id => current_user.id, :order_index => (last_active_dashboard ? last_active_dashboard.order_index+1 : 1))
+      add_default_dashboards_if_first_user_dashboard(@dashboard.global?)
+      last_index=current_user.active_dashboards.max_by(&:order_index).order_index
+      current_user.active_dashboards.create(:dashboard => @dashboard, :user_id => current_user.id, :order_index => last_index+1)
+
       redirect_to :controller => 'dashboard', :action => 'configure', :did => @dashboard.id, :id => (params[:resource] unless @dashboard.global)
     else
       flash[:error]=@dashboard.errors.full_messages.join('<br/>')
+
       redirect_to :action => 'index', :resource => params[:resource]
     end
   end
@@ -116,9 +118,9 @@ class DashboardsController < ApplicationController
   end
 
   def follow
-    add_default_dashboards_if_first_user_dashboard
     dashboard=Dashboard.find(:first, :conditions => ['shared=? and id=? and (user_id is null or user_id<>?)', true, params[:id].to_i, current_user.id])
     if dashboard
+      add_default_dashboards_if_first_user_dashboard(dashboard.global?)
       active_dashboard = current_user.active_dashboards.to_a.find { |ad| ad.name==dashboard.name }
       if active_dashboard
         flash[:error]=Api::Utils.message('dashboard.error_follow_existing_name')
@@ -133,10 +135,9 @@ class DashboardsController < ApplicationController
   end
 
   def unfollow
-    add_default_dashboards_if_first_user_dashboard
-
     dashboard=Dashboard.find(:first, :conditions => ['shared=? and id=? and (user_id is null or user_id<>?)', true, params[:id].to_i, current_user.id])
     if dashboard
+      add_default_dashboards_if_first_user_dashboard(dashboard.global?)
       ActiveDashboard.destroy_all(['user_id=? AND dashboard_id=?', current_user.id, params[:id].to_i])
 
       if ActiveDashboard.count(:conditions => ['user_id=?', current_user.id])==0
@@ -149,18 +150,17 @@ class DashboardsController < ApplicationController
   private
 
   def position(offset)
-    add_default_dashboards_if_first_user_dashboard
-
     dashboards=current_user.active_dashboards.to_a
 
-    to_move = dashboards.find { |a| a.id == params[:id].to_i}
+    to_move = dashboards.find { |a| a.id == params[:id].to_i }
     if to_move
+      add_default_dashboards_if_first_user_dashboard(to_move.global?)
       dashboards_same_type=dashboards.select { |a| (a.global? == to_move.global?) }.sort_by(&:order_index)
 
       index = dashboards_same_type.index(to_move)
       dashboards_same_type[index], dashboards_same_type[index + offset] = dashboards_same_type[index + offset], dashboards_same_type[index]
 
-      dashboards_same_type.each_with_index do |a,i|
+      dashboards_same_type.each_with_index do |a, i|
         a.order_index=i+1
         a.save
       end
@@ -177,11 +177,10 @@ class DashboardsController < ApplicationController
     dashboard.column_layout=Dashboard::DEFAULT_LAYOUT if !dashboard.column_layout
   end
 
-  def add_default_dashboards_if_first_user_dashboard
-    if current_user.active_dashboards.empty?
-      defaults=ActiveDashboard.default_dashboards
-      defaults.each do |default_active|
-        current_user.active_dashboards.create(:dashboard => default_active.dashboard, :user => current_user, :order_index => current_user.active_dashboards.size+1)
+  def add_default_dashboards_if_first_user_dashboard(global)
+    unless current_user.active_dashboards.any? { |a| a.global? == global }
+      ActiveDashboard.default_dashboards.select { |a| a.global? == global }.each do |default_active|
+        current_user.active_dashboards.create(:dashboard => default_active.dashboard, :user => current_user, :order_index => default_active.order_index)
       end
     end
   end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/active_dashboard.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/active_dashboard.rb
index abdbbfa00c8..632a42c63f3 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/active_dashboard.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/active_dashboard.rb
@@ -58,7 +58,7 @@ class ActiveDashboard < ActiveRecord::Base
   def self.user_dashboards(user, global)
     result=nil
     if user && user.id
-      result=find(:all, :include => 'dashboard', :conditions => {:user_id => user.id}, :order => 'order_index').select { |a| a.global? == global}
+      result=find_for_user(user.id).select { |a| a.global? == global}
     end
     if result.nil? || result.empty?
       result=default_dashboards.select { |a| a.global? == global}
@@ -67,6 +67,13 @@ class ActiveDashboard < ActiveRecord::Base
   end
 
   def self.default_dashboards()
-    find(:all, :include => 'dashboard', :conditions => {:user_id => nil}, :order => 'order_index')
+    find_for_user(nil)
   end
+
+  private
+
+  def self.find_for_user(user_id)
+    find(:all, :include => 'dashboard', :conditions => {:user_id => user_id}, :order => 'order_index')
+  end
+
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/dashboard.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/dashboard.rb
index 9341d36d57a..a0ff3cb54ea 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/models/dashboard.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/models/dashboard.rb
@@ -35,16 +35,16 @@ class Dashboard < ActiveRecord::Base
   before_destroy :check_not_default_before_destroy
 
   def name(l10n=false)
-	n=read_attribute(:name)
-	if l10n
-	  Api::Utils.message("dashboard.#{n}.name", :default => n)
-	else
-	  n
-	end
+    n=read_attribute(:name)
+    if l10n
+      Api::Utils.message("dashboard.#{n}.name", :default => n)
+    else
+      n
+    end
   end
 
   def shared?
-	read_attribute(:shared) || false
+    read_attribute(:shared) || false
   end
 
   def global=(global)
@@ -55,71 +55,76 @@ class Dashboard < ActiveRecord::Base
     read_attribute(:is_global)
   end
 
+  def global?
+    global
+  end
+
   def layout
-	column_layout
+    column_layout
   end
 
   def user_name
-	user_id ? user.name : nil
+    user_id ? user.name : nil
   end
 
   def editable_by?(user)
-	(user && self.user_id==user.id) || (user_id.nil? && user.has_role?(:admin))
+    (user && self.user_id==user.id) || (user_id.nil? && user.has_role?(:admin))
   end
 
   def owner?(user)
-	self.user_id==user.id
+    self.user_id==user.id
   end
 
   def number_of_columns
-	column_layout.split('-').size
+    column_layout.split('-').size
   end
 
   def column_size(column_index)
-	last_widget=widgets.select { |w| w.column_index==column_index }.max { |x, y| x.row_index <=> y.row_index }
-	last_widget ? last_widget.row_index : 0
+    last_widget=widgets.select { |w| w.column_index==column_index }.max { |x, y| x.row_index <=> y.row_index }
+    last_widget ? last_widget.row_index : 0
   end
 
   def deep_copy()
-	dashboard=Dashboard.new(attributes)
-	dashboard.shared=false
-	self.widgets.each do |child|
-	  new_widget = Widget.create(child.attributes)
-
-	  child.properties.each do |prop|
-		widget_prop = WidgetProperty.create(prop.attributes)
-		new_widget.properties << widget_prop
-	  end
-
-	  new_widget.save
-	  dashboard.widgets << new_widget
-	end
-	dashboard.save
-	dashboard
+    dashboard=Dashboard.new(attributes)
+    dashboard.shared=false
+    self.widgets.each do |child|
+      new_widget = Widget.create(child.attributes)
+
+      child.properties.each do |prop|
+        widget_prop = WidgetProperty.create(prop.attributes)
+        new_widget.properties << widget_prop
+      end
+
+      new_widget.save
+      dashboard.widgets << new_widget
+    end
+    dashboard.save
+    dashboard
   end
 
   def provided_programmatically?
-	shared && user_id.nil?
+    shared && user_id.nil?
   end
 
   protected
   def check_not_default_before_destroy
-	if shared?
-	  default_actives = active_dashboards.select { |ad| ad.default? }
-	  return default_actives.size==0
-	end
-	true
+    if shared?
+      default_actives = active_dashboards.select { |ad| ad.default? }
+      return default_actives.size==0
+    end
+    true
   end
 
   def validate_on_update
-	# Check that not used as default dashboard when unsharing
-	if shared_was && !shared
-	  # unsharing
-	  default_actives = active_dashboards.select { |ad| ad.default? }
-
-	  unless default_actives.empty?
-		errors.add_to_base(Api::Utils.message('dashboard.error_unshare_default'))
-	  end
-	end
+    # Check that not used as default dashboard when unsharing
+    if shared_was && !shared
+      # unsharing
+      default_actives = active_dashboards.select { |ad| ad.default? }
+
+      unless default_actives.empty?
+        errors.add_to_base(Api::Utils.message('dashboard.error_unshare_default'))
+      end
+    end
   end
+
 end
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/dashboards/index.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/dashboards/index.html.erb
index 7149e5770a8..91353957c0b 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/views/dashboards/index.html.erb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/views/dashboards/index.html.erb
@@ -56,7 +56,9 @@
                 <% end %>
               <% end %>
               <% if @actives.size() > 1 and active.shared? and !active.dashboard.owner?(current_user) %>
+                <% if active.editable_by?(current_user) %>
                 |
+                <% end %>
                 <%= link_to message('unfollow'), {:action => :unfollow, :id => active.dashboard_id, :resource => params[:resource]}, :method => :post,
                             :id => "unfollow-#{u active.name}", :class => 'link-action' %>
               <% end %>