From 9b1ebd6808d260a95eff38c2a93ec2c1b7ed8183 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 23 Feb 2013 10:53:21 +0000 Subject: [PATCH] Refactor: makes issue id a regular QueryColumn. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@11447 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/helpers/issues_helper.rb | 4 ++-- app/helpers/queries_helper.rb | 4 +++- app/models/issue_query.rb | 5 +---- app/models/query.rb | 12 +++++++--- app/views/issues/_list.html.erb | 4 +--- app/views/queries/_columns.html.erb | 4 ++-- test/functional/issues_controller_test.rb | 14 ++++++------ test/functional/queries_controller_test.rb | 2 +- test/unit/query_test.rb | 26 ++++++++++++++++++---- 9 files changed, 48 insertions(+), 27 deletions(-) diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 73ea79cb7..a0c48e20d 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -382,10 +382,10 @@ module IssuesHelper export = FCSV.generate(:col_sep => l(:general_csv_separator)) do |csv| # csv header fields - csv << [ "#" ] + columns.collect {|c| Redmine::CodesetUtil.from_utf8(c.caption.to_s, encoding) } + csv << columns.collect {|c| Redmine::CodesetUtil.from_utf8(c.caption.to_s, encoding) } # csv lines issues.each do |issue| - csv << [ issue.id.to_s ] + columns.collect {|c| Redmine::CodesetUtil.from_utf8(csv_content(c, issue), encoding) } + csv << columns.collect {|c| Redmine::CodesetUtil.from_utf8(csv_content(c, issue), encoding) } end end export diff --git a/app/helpers/queries_helper.rb b/app/helpers/queries_helper.rb index d140ab3a3..80bceaf10 100644 --- a/app/helpers/queries_helper.rb +++ b/app/helpers/queries_helper.rb @@ -67,7 +67,9 @@ module QueriesHelper when 'Date' format_date(value) when 'Fixnum' - if column.name == :done_ratio + if column.name == :id + link_to value, issue_path(issue) + elsif column.name == :done_ratio progress_bar(value, :width => '80px') else value.to_s diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb index 3ca31db9a..5b818cc28 100644 --- a/app/models/issue_query.rb +++ b/app/models/issue_query.rb @@ -20,6 +20,7 @@ class IssueQuery < Query self.queried_class = Issue self.available_columns = [ + QueryColumn.new(:id, :sortable => "#{Issue.table_name}.id", :default_order => 'desc', :caption => '#', :frozen => true), QueryColumn.new(:project, :sortable => "#{Project.table_name}.name", :groupable => true), QueryColumn.new(:tracker, :sortable => "#{Tracker.table_name}.position", :groupable => true), QueryColumn.new(:parent, :sortable => ["#{Issue.table_name}.root_id", "#{Issue.table_name}.lft ASC"], :default_order => 'desc', :caption => :field_parent_issue), @@ -216,10 +217,6 @@ class IssueQuery < Query @available_columns end - def sortable_columns - {'id' => "#{Issue.table_name}.id"}.merge(super) - end - def default_columns_names @default_columns_names ||= begin default_columns = Setting.issue_list_default_columns.map(&:to_sym) diff --git a/app/models/query.rb b/app/models/query.rb index cd1b9c386..db6ca07a5 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -28,11 +28,12 @@ class QueryColumn end self.default_order = options[:default_order] @inline = options.key?(:inline) ? options[:inline] : true - @caption_key = options[:caption] || "field_#{name}" + @caption_key = options[:caption] || "field_#{name}".to_sym + @frozen = options[:frozen] end def caption - l(@caption_key) + @caption_key.is_a?(Symbol) ? l(@caption_key) : @caption_key end # Returns true if the column is sortable, otherwise false @@ -48,6 +49,10 @@ class QueryColumn @inline end + def frozen? + @frozen + end + def value(object) object.send name end @@ -382,9 +387,10 @@ class Query < ActiveRecord::Base def columns # preserve the column_names order - (has_default_columns? ? default_columns_names : column_names).collect do |name| + cols = (has_default_columns? ? default_columns_names : column_names).collect do |name| available_columns.find { |col| col.name == name } end.compact + available_columns.select(&:frozen?) | cols end def inline_columns diff --git a/app/views/issues/_list.html.erb b/app/views/issues/_list.html.erb index f3e8cae2c..17bf99623 100644 --- a/app/views/issues/_list.html.erb +++ b/app/views/issues/_list.html.erb @@ -9,7 +9,6 @@ :onclick => 'toggleIssuesSelection(this); return false;', :title => "#{l(:button_check_all)}/#{l(:button_uncheck_all)}" %> - <%= sort_header_tag('id', :caption => '#', :default_order => 'desc') %> <% query.inline_columns.each do |column| %> <%= column_header(column) %> <% end %> @@ -32,13 +31,12 @@ <% end %> "> <%= check_box_tag("ids[]", issue.id, false, :id => nil) %> - <%= link_to issue.id, issue_path(issue) %> <%= raw query.inline_columns.map {|column| "#{column_content(column, issue)}"}.join %> <% @query.block_columns.each do |column| if (text = column_content(column, issue)) && text.present? -%> - <%= text %> + <%= text %> <% end -%> <% end -%> diff --git a/app/views/queries/_columns.html.erb b/app/views/queries/_columns.html.erb index 78d3c5703..329ffd67c 100644 --- a/app/views/queries/_columns.html.erb +++ b/app/views/queries/_columns.html.erb @@ -4,7 +4,7 @@ <%= label_tag "available_columns", l(:description_available_columns) %>
<%= select_tag 'available_columns', - options_for_select((query.available_inline_columns - query.columns).collect {|column| [column.caption, column.name]}), + options_for_select((query.available_inline_columns - query.columns).reject(&:frozen?).collect {|column| [column.caption, column.name]}), :multiple => true, :size => 10, :style => "width:150px", :ondblclick => "moveOptions(this.form.available_columns, this.form.selected_columns);" %> @@ -18,7 +18,7 @@ <%= label_tag "selected_columns", l(:description_selected_columns) %>
<%= select_tag((defined?(tag_name) ? tag_name : 'c[]'), - options_for_select(query.inline_columns.collect {|column| [column.caption, column.name]}), + options_for_select((query.inline_columns & query.available_inline_columns).reject(&:frozen?).collect {|column| [column.caption, column.name]}), :id => 'selected_columns', :multiple => true, :size => 10, :style => "width:150px", :ondblclick => "moveOptions(this.form.selected_columns, this.form.available_columns);") %> diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 65e28b570..df3c2e330 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -380,7 +380,7 @@ class IssuesControllerTest < ActionController::TestCase assert_equal 'text/csv; header=present', @response.content_type assert @response.body.starts_with?("#,") lines = @response.body.chomp.split("\n") - assert_equal assigns(:query).columns.size + 1, lines[0].split(',').size + assert_equal assigns(:query).columns.size, lines[0].split(',').size end def test_index_csv_with_project @@ -397,7 +397,7 @@ class IssuesControllerTest < ActionController::TestCase assert_equal 'text/csv; header=present', @response.content_type assert @response.body.starts_with?("#,") lines = @response.body.chomp.split("\n") - assert_equal assigns(:query).columns.size + 2, lines[0].split(',').size + assert_equal assigns(:query).columns.size + 1, lines[0].split(',').size end def test_index_csv_with_spent_time_column @@ -416,9 +416,9 @@ class IssuesControllerTest < ActionController::TestCase assert_response :success assert_not_nil assigns(:issues) assert_equal 'text/csv; header=present', @response.content_type - assert @response.body.starts_with?("#,") - lines = @response.body.chomp.split("\n") - assert_equal assigns(:query).available_inline_columns.size + 1, lines[0].split(',').size + assert_match /\A#,/, response.body + lines = response.body.chomp.split("\n") + assert_equal assigns(:query).available_inline_columns.size, lines[0].split(',').size end def test_index_csv_with_multi_column_field @@ -708,12 +708,12 @@ class IssuesControllerTest < ActionController::TestCase # query should use specified columns query = assigns(:query) assert_kind_of IssueQuery, query - assert_equal [:project, :tracker, :subject, :assigned_to], query.columns.map(&:name) + assert_equal [:id, :project, :tracker, :subject, :assigned_to], query.columns.map(&:name) end def test_index_without_project_and_explicit_default_columns_should_not_add_project_column Setting.issue_list_default_columns = ['tracker', 'subject', 'assigned_to'] - columns = ['tracker', 'subject', 'assigned_to'] + columns = ['id', 'tracker', 'subject', 'assigned_to'] get :index, :set_filter => 1, :c => columns # query should use specified columns diff --git a/test/functional/queries_controller_test.rb b/test/functional/queries_controller_test.rb index 8a4e49182..a232fec2e 100644 --- a/test/functional/queries_controller_test.rb +++ b/test/functional/queries_controller_test.rb @@ -114,7 +114,7 @@ class QueriesControllerTest < ActionController::TestCase assert_redirected_to :controller => 'issues', :action => 'index', :project_id => nil, :query_id => q assert !q.is_public? assert !q.has_default_columns? - assert_equal [:tracker, :subject, :priority, :category], q.columns.collect {|c| c.name} + assert_equal [:id, :tracker, :subject, :priority, :category], q.columns.collect {|c| c.name} assert q.valid? end diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 0ee76a477..bf194155e 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -750,16 +750,34 @@ class QueryTest < ActiveSupport::TestCase def test_set_column_names q = IssueQuery.new q.column_names = ['tracker', :subject, '', 'unknonw_column'] - assert_equal [:tracker, :subject], q.columns.collect {|c| c.name} - c = q.columns.first - assert q.has_column?(c) + assert_equal [:id, :tracker, :subject], q.columns.collect {|c| c.name} + end + + def test_has_column_should_accept_a_column_name + q = IssueQuery.new + q.column_names = ['tracker', :subject] + assert q.has_column?(:tracker) + assert !q.has_column?(:category) + end + + def test_has_column_should_accept_a_column + q = IssueQuery.new + q.column_names = ['tracker', :subject] + + tracker_column = q.available_columns.detect {|c| c.name==:tracker} + assert_kind_of QueryColumn, tracker_column + category_column = q.available_columns.detect {|c| c.name==:category} + assert_kind_of QueryColumn, category_column + + assert q.has_column?(tracker_column) + assert !q.has_column?(category_column) end def test_inline_and_block_columns q = IssueQuery.new q.column_names = ['subject', 'description', 'tracker'] - assert_equal [:subject, :tracker], q.inline_columns.map(&:name) + assert_equal [:id, :subject, :tracker], q.inline_columns.map(&:name) assert_equal [:description], q.block_columns.map(&:name) end -- 2.39.5