From 8b4e95d423875d4317a4777f4c972c571fd2e291 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 24 May 2012 10:13:12 +0200 Subject: [PATCH] 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 --- .../admin_dashboards_controller.rb | 42 +++------ .../app/controllers/dashboards_controller.rb | 39 ++++---- .../WEB-INF/app/models/active_dashboard.rb | 11 ++- .../webapp/WEB-INF/app/models/dashboard.rb | 93 ++++++++++--------- .../app/views/dashboards/index.html.erb | 2 + 5 files changed, 90 insertions(+), 97 deletions(-) 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('
') + 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 %> -- 2.39.5