From: Jean-Philippe Lang Date: Sun, 29 Jan 2012 20:51:48 +0000 (+0000) Subject: Adds support for multiselect custom fields (#1189). X-Git-Tag: 1.4.0~574 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=cd6db6a3cbe43880eca4eec8c967fb78d95a2926;p=redmine.git Adds support for multiselect custom fields (#1189). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8721 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/app/helpers/custom_fields_helper.rb b/app/helpers/custom_fields_helper.rb index 7b1927c0a..639af95fb 100644 --- a/app/helpers/custom_fields_helper.rb +++ b/app/helpers/custom_fields_helper.rb @@ -36,6 +36,7 @@ module CustomFieldsHelper def custom_field_tag(name, custom_value) custom_field = custom_value.custom_field field_name = "#{name}[custom_field_values][#{custom_field.id}]" + field_name << "[]" if custom_field.multiple? field_id = "#{name}_custom_field_values_#{custom_field.id}" field_format = Redmine::CustomFieldFormat.find_by_name(custom_field.field_format) @@ -48,10 +49,22 @@ module CustomFieldsHelper when "bool" hidden_field_tag(field_name, '0') + check_box_tag(field_name, '1', custom_value.true?, :id => field_id) when "list" - blank_option = custom_field.is_required? ? - (custom_field.default_value.blank? ? "" : '') : - '' - select_tag(field_name, blank_option.html_safe + options_for_select(custom_field.possible_values_options(custom_value.customized), custom_value.value), :id => field_id) + blank_option = '' + unless custom_field.multiple? + if custom_field.is_required? + unless custom_field.default_value.present? + blank_option = "" + end + else + blank_option = '' + end + end + s = select_tag(field_name, blank_option.html_safe + options_for_select(custom_field.possible_values_options(custom_value.customized), custom_value.value), + :id => field_id, :multiple => custom_field.multiple?) + if custom_field.multiple? + s << hidden_field_tag(field_name, '') + end + s else text_field_tag(field_name, custom_value.value, :id => field_id) end @@ -71,6 +84,7 @@ module CustomFieldsHelper def custom_field_tag_for_bulk_edit(name, custom_field, projects=nil) field_name = "#{name}[custom_field_values][#{custom_field.id}]" + field_name << "[]" if custom_field.multiple? field_id = "#{name}_custom_field_values_#{custom_field.id}" field_format = Redmine::CustomFieldFormat.find_by_name(custom_field.field_format) case field_format.try(:edit_as) @@ -84,7 +98,11 @@ module CustomFieldsHelper [l(:general_text_yes), '1'], [l(:general_text_no), '0']]), :id => field_id) when "list" - select_tag(field_name, options_for_select([[l(:label_no_change_option), '']] + custom_field.possible_values_options(projects)), :id => field_id) + options = [] + options << [l(:label_no_change_option), ''] unless custom_field.multiple? + options += custom_field.possible_values_options(projects) + select_tag(field_name, options_for_select(options), + :id => field_id, :multiple => custom_field.multiple?) else text_field_tag(field_name, '', :id => field_id) end @@ -98,7 +116,11 @@ module CustomFieldsHelper # Return a string used to display a custom value def format_value(value, field_format) - Redmine::CustomFieldFormat.format_value(value, field_format) # Proxy + if value.is_a?(Array) + value.collect {|v| format_value(v, field_format)}.join(', ') + else + Redmine::CustomFieldFormat.format_value(value, field_format) + end end # Return an array of custom field formats which can be used in select_tag @@ -110,8 +132,18 @@ module CustomFieldsHelper def render_api_custom_values(custom_values, api) api.array :custom_fields do custom_values.each do |custom_value| - api.custom_field :id => custom_value.custom_field_id, :name => custom_value.custom_field.name do - api.value custom_value.value + attrs = {:id => custom_value.custom_field_id, :name => custom_value.custom_field.name} + attrs.merge!(:multiple => true) if custom_value.custom_field.multiple? + api.custom_field attrs do + if custom_value.value.is_a?(Array) + api.array :value do + custom_value.value.each do |value| + api.value value unless value.blank? + end + end + else + api.value custom_value.value + end end end end unless custom_values.empty? diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 6c0a8b4e5..e0e47e414 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -161,7 +161,44 @@ module IssuesHelper out end + # Returns the textual representation of a journal details + # as an array of strings + def details_to_strings(details, no_html=false) + strings = [] + values_by_field = {} + details.each do |detail| + if detail.property == 'cf' + field_id = detail.prop_key + field = CustomField.find_by_id(field_id) + if field && field.multiple? + values_by_field[field_id] ||= {:added => [], :deleted => []} + if detail.old_value + values_by_field[field_id][:deleted] << detail.old_value + end + if detail.value + values_by_field[field_id][:added] << detail.value + end + next + end + end + strings << show_detail(detail, no_html) + end + values_by_field.each do |field_id, changes| + detail = JournalDetail.new(:property => 'cf', :prop_key => field_id) + if changes[:added].any? + detail.value = changes[:added] + strings << show_detail(detail, no_html) + elsif changes[:deleted].any? + detail.old_value = changes[:deleted] + strings << show_detail(detail, no_html) + end + end + strings + end + + # Returns the textual representation of a single journal detail def show_detail(detail, no_html=false) + multiple = false case detail.property when 'attr' field = detail.prop_key.to_s.gsub(/\_id$/, "") @@ -192,6 +229,7 @@ module IssuesHelper when 'cf' custom_field = CustomField.find_by_id(detail.prop_key) if custom_field + multiple = custom_field.multiple? label = custom_field.name value = format_value(detail.value, custom_field.field_format) if detail.value old_value = format_value(detail.old_value, custom_field.field_format) if detail.old_value @@ -232,6 +270,8 @@ module IssuesHelper when 'attr', 'cf' if !detail.old_value.blank? l(:text_journal_changed, :label => label, :old => old_value, :new => value).html_safe + elsif multiple + l(:text_journal_added, :label => label, :value => value).html_safe else l(:text_journal_set_to, :label => label, :value => value).html_safe end diff --git a/app/helpers/queries_helper.rb b/app/helpers/queries_helper.rb index 681a19a45..bead33315 100644 --- a/app/helpers/queries_helper.rb +++ b/app/helpers/queries_helper.rb @@ -31,7 +31,14 @@ module QueriesHelper def column_content(column, issue) value = column.value(issue) - + if value.is_a?(Array) + value.collect {|v| column_value(column, issue, v)}.compact.sort.join(', ') + else + column_value(column, issue, value) + end + end + + def column_value(column, issue, value) case value.class.name when 'String' if column.name == :subject diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb index 0dd70de1d..f56eb3623 100644 --- a/app/models/custom_field.rb +++ b/app/models/custom_field.rb @@ -38,6 +38,8 @@ class CustomField < ActiveRecord::Base def set_searchable # make sure these fields are not searchable self.searchable = false if %w(int float date bool).include?(field_format) + # make sure only these fields can have multiple values + self.multiple = false unless %w(list user version).include?(field_format) true end @@ -123,6 +125,7 @@ class CustomField < ActiveRecord::Base # objects by their value of the custom field. # Returns false, if the custom field can not be used for sorting. def order_statement + return nil if multiple? case field_format when 'string', 'text', 'list', 'date', 'bool' # COALESCE is here to make sure that blank and NULL values are sorted equally @@ -161,14 +164,24 @@ class CustomField < ActiveRecord::Base nil end - # Returns the error message for the given value + # Returns the error messages for the given value # or an empty array if value is a valid value for the custom field def validate_field_value(value) errs = [] - if is_required? && value.blank? - errs << ::I18n.t('activerecord.errors.messages.blank') + if value.is_a?(Array) + if !multiple? + errs << ::I18n.t('activerecord.errors.messages.invalid') + end + if is_required? && value.detect(&:present?).nil? + errs << ::I18n.t('activerecord.errors.messages.blank') + end + value.each {|v| errs += validate_field_value_format(v)} + else + if is_required? && value.blank? + errs << ::I18n.t('activerecord.errors.messages.blank') + end + errs += validate_field_value_format(value) end - errs += validate_field_value_format(value) errs end diff --git a/app/models/issue.rb b/app/models/issue.rb index d20cff35d..f6c458990 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -429,7 +429,7 @@ class Issue < ActiveRecord::Base else @attributes_before_change = attributes.dup @custom_values_before_change = {} - self.custom_values.each {|c| @custom_values_before_change.store c.custom_field_id, c.value } + self.custom_field_values.each {|c| @custom_values_before_change.store c.custom_field_id, c.value } end # Make sure updated_on is updated when adding a note. updated_on_will_change! @@ -1006,14 +1006,35 @@ class Issue < ActiveRecord::Base end if @custom_values_before_change # custom fields changes - custom_values.each {|c| + custom_field_values.each {|c| before = @custom_values_before_change[c.custom_field_id] after = c.value next if before == after || (before.blank? && after.blank?) - @current_journal.details << JournalDetail.new(:property => 'cf', - :prop_key => c.custom_field_id, - :old_value => before, - :value => after) + + if before.is_a?(Array) || after.is_a?(Array) + before = [before] unless before.is_a?(Array) + after = [after] unless after.is_a?(Array) + + # values removed + (before - after).reject(&:blank?).each do |value| + @current_journal.details << JournalDetail.new(:property => 'cf', + :prop_key => c.custom_field_id, + :old_value => value, + :value => nil) + end + # values added + (after - before).reject(&:blank?).each do |value| + @current_journal.details << JournalDetail.new(:property => 'cf', + :prop_key => c.custom_field_id, + :old_value => nil, + :value => value) + end + else + @current_journal.details << JournalDetail.new(:property => 'cf', + :prop_key => c.custom_field_id, + :old_value => before, + :value => after) + end } end @current_journal.save diff --git a/app/models/query.rb b/app/models/query.rb index 9846acd63..1cfa952ff 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -57,7 +57,7 @@ class QueryCustomFieldColumn < QueryColumn def initialize(custom_field) self.name = "cf_#{custom_field.id}".to_sym self.sortable = custom_field.order_statement || false - if %w(list date bool int).include?(custom_field.field_format) + if %w(list date bool int).include?(custom_field.field_format) && !custom_field.multiple? self.groupable = custom_field.order_statement end self.groupable ||= false @@ -73,8 +73,8 @@ class QueryCustomFieldColumn < QueryColumn end def value(issue) - cv = issue.custom_values.detect {|v| v.custom_field_id == @cf.id} - cv && @cf.cast_value(cv.value) + cv = issue.custom_values.select {|v| v.custom_field_id == @cf.id}.collect {|v| @cf.cast_value(v.value)} + cv.size > 1 ? cv : cv.first end def css_classes @@ -694,7 +694,13 @@ class Query < ActiveRecord::Base value.push User.current.id.to_s end end - "#{Issue.table_name}.id IN (SELECT #{Issue.table_name}.id FROM #{Issue.table_name} LEFT OUTER JOIN #{db_table} ON #{db_table}.customized_type='Issue' AND #{db_table}.customized_id=#{Issue.table_name}.id AND #{db_table}.custom_field_id=#{custom_field_id} WHERE " + + not_in = nil + if operator == '!' + # Makes ! operator work for custom fields with multiple values + operator = '=' + not_in = 'NOT' + end + "#{Issue.table_name}.id #{not_in} IN (SELECT #{Issue.table_name}.id FROM #{Issue.table_name} LEFT OUTER JOIN #{db_table} ON #{db_table}.customized_type='Issue' AND #{db_table}.customized_id=#{Issue.table_name}.id AND #{db_table}.custom_field_id=#{custom_field_id} WHERE " + sql_for_field(field, operator, value, db_table, db_field, true) + ')' end diff --git a/app/views/custom_fields/_form.html.erb b/app/views/custom_fields/_form.html.erb index a5abb2031..d8efca953 100644 --- a/app/views/custom_fields/_form.html.erb +++ b/app/views/custom_fields/_form.html.erb @@ -9,6 +9,7 @@ function toggle_custom_field_format() { p_values = $("custom_field_possible_values"); p_searchable = $("custom_field_searchable"); p_default = $("custom_field_default_value"); + p_multiple = $("custom_field_multiple"); p_default.setAttribute('type','text'); Element.show(p_default.parentNode); @@ -19,6 +20,7 @@ function toggle_custom_field_format() { Element.hide(p_regexp.parentNode); if (p_searchable) Element.show(p_searchable.parentNode); Element.show(p_values.parentNode); + Element.show(p_multiple.parentNode); break; case "bool": p_default.setAttribute('type','checkbox'); @@ -26,12 +28,14 @@ function toggle_custom_field_format() { Element.hide(p_regexp.parentNode); if (p_searchable) Element.hide(p_searchable.parentNode); Element.hide(p_values.parentNode); + Element.hide(p_multiple.parentNode); break; case "date": Element.hide(p_length.parentNode); Element.hide(p_regexp.parentNode); if (p_searchable) Element.hide(p_searchable.parentNode); Element.hide(p_values.parentNode); + Element.hide(p_multiple.parentNode); break; case "float": case "int": @@ -39,6 +43,7 @@ function toggle_custom_field_format() { Element.show(p_regexp.parentNode); if (p_searchable) Element.hide(p_searchable.parentNode); Element.hide(p_values.parentNode); + Element.hide(p_multiple.parentNode); break; case "user": case "version": @@ -47,12 +52,14 @@ function toggle_custom_field_format() { if (p_searchable) Element.hide(p_searchable.parentNode); Element.hide(p_values.parentNode); Element.hide(p_default.parentNode); + Element.show(p_multiple.parentNode); break; default: Element.show(p_length.parentNode); Element.show(p_regexp.parentNode); if (p_searchable) Element.show(p_searchable.parentNode); Element.hide(p_values.parentNode); + Element.hide(p_multiple.parentNode); break; } } @@ -64,6 +71,7 @@ function toggle_custom_field_format() {

<%= f.text_field :name, :required => true %>

<%= f.select :field_format, custom_field_formats_for_select(@custom_field), {}, :onchange => "toggle_custom_field_format();", :disabled => !@custom_field.new_record? %>

+

<%= f.check_box :multiple, :disabled => !@custom_field.new_record? %>

<%= f.text_field :min_length, :size => 5, :no_label => true %> - <%= f.text_field :max_length, :size => 5, :no_label => true %>
(<%=l(:text_min_max_length_info)%>)

diff --git a/app/views/issues/_history.html.erb b/app/views/issues/_history.html.erb index d5853c164..aea0d1e1f 100644 --- a/app/views/issues/_history.html.erb +++ b/app/views/issues/_history.html.erb @@ -8,8 +8,8 @@ <% if journal.details.any? %> <% end %> diff --git a/app/views/journals/index.builder b/app/views/journals/index.builder index e70777757..a81ff98a8 100644 --- a/app/views/journals/index.builder +++ b/app/views/journals/index.builder @@ -19,8 +19,8 @@ xml.feed "xmlns" => "http://www.w3.org/2005/Atom" do end xml.content "type" => "html" do xml.text! '' xml.text! textilizable(change, :notes, :only_path => false) unless change.notes.blank? diff --git a/app/views/mailer/issue_edit.html.erb b/app/views/mailer/issue_edit.html.erb index 53bd089e6..07288e379 100644 --- a/app/views/mailer/issue_edit.html.erb +++ b/app/views/mailer/issue_edit.html.erb @@ -1,8 +1,8 @@ <%= l(:text_issue_updated, :id => "##{@issue.id}", :author => h(@journal.user)) %> diff --git a/app/views/mailer/issue_edit.text.erb b/app/views/mailer/issue_edit.text.erb index 1ea2a64e5..1f7e7dc32 100644 --- a/app/views/mailer/issue_edit.text.erb +++ b/app/views/mailer/issue_edit.text.erb @@ -1,7 +1,7 @@ <%= l(:text_issue_updated, :id => "##{@issue.id}", :author => @journal.user) %> -<% for detail in @journal.details -%> -<%= show_detail(detail, true) %> +<% details_to_strings(@journal.details, true).each do |string| -%> +<%= string %> <% end -%> <%= @journal.notes if @journal.notes? %> diff --git a/config/locales/en.yml b/config/locales/en.yml index e9f160db1..90266c2ca 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -319,6 +319,7 @@ en: field_cvsroot: CVSROOT field_cvs_module: Module field_repository_is_default: Main repository + field_multiple: Multiple values setting_app_title: Application title setting_app_subtitle: Application subtitle diff --git a/config/locales/fr.yml b/config/locales/fr.yml index e2029b591..17dc8a9be 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -318,6 +318,7 @@ fr: field_is_private: Privée field_commit_logs_encoding: Encodage des messages de commit field_repository_is_default: Dépôt principal + field_multiple: Valeurs multiples setting_app_title: Titre de l'application setting_app_subtitle: Sous-titre de l'application diff --git a/db/migrate/20120127174243_add_custom_fields_multiple.rb b/db/migrate/20120127174243_add_custom_fields_multiple.rb new file mode 100644 index 000000000..caee40bd4 --- /dev/null +++ b/db/migrate/20120127174243_add_custom_fields_multiple.rb @@ -0,0 +1,9 @@ +class AddCustomFieldsMultiple < ActiveRecord::Migration + def self.up + add_column :custom_fields, :multiple, :boolean, :default => false + end + + def self.down + remove_column :custom_fields, :multiple + end +end diff --git a/lib/redmine/export/pdf.rb b/lib/redmine/export/pdf.rb index 1da58855b..8ae7575de 100644 --- a/lib/redmine/export/pdf.rb +++ b/lib/redmine/export/pdf.rb @@ -464,8 +464,8 @@ module Redmine " - " + journal.user.name) pdf.Ln pdf.SetFontStyle('I',8) - for detail in journal.details - pdf.RDMMultiCell(190,5, "- " + show_detail(detail, true)) + details_to_strings(journal.details, true).each do |string| + pdf.RDMMultiCell(190,5, "- " + string) end if journal.notes? pdf.Ln unless journal.details.empty? diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 129faad1e..aa96156b0 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -625,6 +625,36 @@ class IssuesControllerTest < ActionController::TestCase :ancestor => {:tag => 'table', :attributes => {:class => /issues/}} end + def test_index_with_multi_custom_field_column + field = CustomField.find(1) + field.update_attribute :multiple, true + issue = Issue.find(1) + issue.custom_field_values = {1 => ['MySQL', 'Oracle']} + issue.save! + + get :index, :set_filter => 1, :c => %w(tracker subject cf_1) + assert_response :success + + assert_tag :td, + :attributes => {:class => /cf_1/}, + :content => 'MySQL, Oracle' + end + + def test_index_with_multi_user_custom_field_column + field = IssueCustomField.create!(:name => 'Multi user', :field_format => 'user', :multiple => true, + :tracker_ids => [1], :is_for_all => true) + issue = Issue.find(1) + issue.custom_field_values = {field.id => ['2', '3']} + issue.save! + + get :index, :set_filter => 1, :c => ['tracker', 'subject', "cf_#{field.id}"] + assert_response :success + + assert_tag :td, + :attributes => {:class => /cf_#{field.id}/}, + :child => {:tag => 'a', :content => 'John Smith'} + end + def test_index_with_date_column Issue.find(1).update_attribute :start_date, '1987-08-24' @@ -1032,6 +1062,33 @@ class IssuesControllerTest < ActionController::TestCase assert_no_tag 'a', :content => /Next/ end + def test_show_with_multi_custom_field + field = CustomField.find(1) + field.update_attribute :multiple, true + issue = Issue.find(1) + issue.custom_field_values = {1 => ['MySQL', 'Oracle']} + issue.save! + + get :show, :id => 1 + assert_response :success + + assert_tag :td, :content => 'MySQL, Oracle' + end + + def test_show_with_multi_user_custom_field + field = IssueCustomField.create!(:name => 'Multi user', :field_format => 'user', :multiple => true, + :tracker_ids => [1], :is_for_all => true) + issue = Issue.find(1) + issue.custom_field_values = {field.id => ['2', '3']} + issue.save! + + get :show, :id => 1 + assert_response :success + + # TODO: should display links + assert_tag :td, :content => 'John Smith, Dave Lopper' + end + def test_show_atom get :show, :id => 2, :format => 'atom' assert_response :success @@ -1104,6 +1161,40 @@ class IssuesControllerTest < ActionController::TestCase assert_no_tag 'input', :attributes => {:name => 'issue[watcher_user_ids][]'} end + def test_get_new_with_multi_custom_field + field = IssueCustomField.find(1) + field.update_attribute :multiple, true + + @request.session[:user_id] = 2 + get :new, :project_id => 1, :tracker_id => 1 + assert_response :success + assert_template 'new' + + assert_tag 'select', + :attributes => {:name => 'issue[custom_field_values][1][]', :multiple => 'multiple'}, + :children => {:count => 3}, + :child => {:tag => 'option', :attributes => {:value => 'MySQL'}, :content => 'MySQL'} + assert_tag 'input', + :attributes => {:name => 'issue[custom_field_values][1][]', :value => ''} + end + + def test_get_new_with_multi_user_custom_field + field = IssueCustomField.create!(:name => 'Multi user', :field_format => 'user', :multiple => true, + :tracker_ids => [1], :is_for_all => true) + + @request.session[:user_id] = 2 + get :new, :project_id => 1, :tracker_id => 1 + assert_response :success + assert_template 'new' + + assert_tag 'select', + :attributes => {:name => "issue[custom_field_values][#{field.id}][]", :multiple => 'multiple'}, + :children => {:count => Project.find(1).users.count}, + :child => {:tag => 'option', :attributes => {:value => '2'}, :content => 'John Smith'} + assert_tag 'input', + :attributes => {:name => "issue[custom_field_values][#{field.id}][]", :value => ''} + end + def test_get_new_without_default_start_date_is_creation_date Setting.default_issue_start_date_to_creation_date = 0 @@ -1303,6 +1394,60 @@ class IssuesControllerTest < ActionController::TestCase assert_redirected_to :controller => 'issues', :action => 'show', :id => Issue.last.id end + def test_post_create_with_multi_custom_field + field = IssueCustomField.find_by_name('Database') + field.update_attribute(:multiple, true) + + @request.session[:user_id] = 2 + assert_difference 'Issue.count' do + post :create, :project_id => 1, + :issue => {:tracker_id => 1, + :subject => 'This is the test_new issue', + :description => 'This is the description', + :priority_id => 5, + :custom_field_values => {'1' => ['', 'MySQL', 'Oracle']}} + end + assert_response 302 + issue = Issue.first(:order => 'id DESC') + assert_equal ['MySQL', 'Oracle'], issue.custom_field_value(1).sort + end + + def test_post_create_with_empty_multi_custom_field + field = IssueCustomField.find_by_name('Database') + field.update_attribute(:multiple, true) + + @request.session[:user_id] = 2 + assert_difference 'Issue.count' do + post :create, :project_id => 1, + :issue => {:tracker_id => 1, + :subject => 'This is the test_new issue', + :description => 'This is the description', + :priority_id => 5, + :custom_field_values => {'1' => ['']}} + end + assert_response 302 + issue = Issue.first(:order => 'id DESC') + assert_equal [''], issue.custom_field_value(1).sort + end + + def test_post_create_with_multi_user_custom_field + field = IssueCustomField.create!(:name => 'Multi user', :field_format => 'user', :multiple => true, + :tracker_ids => [1], :is_for_all => true) + + @request.session[:user_id] = 2 + assert_difference 'Issue.count' do + post :create, :project_id => 1, + :issue => {:tracker_id => 1, + :subject => 'This is the test_new issue', + :description => 'This is the description', + :priority_id => 5, + :custom_field_values => {field.id.to_s => ['', '2', '3']}} + end + assert_response 302 + issue = Issue.first(:order => 'id DESC') + assert_equal ['2', '3'], issue.custom_field_value(field).sort + end + def test_post_create_with_required_custom_field_and_without_custom_fields_param field = IssueCustomField.find_by_name('Database') field.update_attribute(:is_required, true) @@ -1822,6 +1967,27 @@ class IssuesControllerTest < ActionController::TestCase assert_tag :input, :attributes => { :name => 'time_entry[comments]', :value => 'test_get_edit_with_params' } end + def test_get_edit_with_multi_custom_field + field = CustomField.find(1) + field.update_attribute :multiple, true + issue = Issue.find(1) + issue.custom_field_values = {1 => ['MySQL', 'Oracle']} + issue.save! + + @request.session[:user_id] = 2 + get :edit, :id => 1 + assert_response :success + assert_template 'edit' + + assert_tag 'select', :attributes => {:name => 'issue[custom_field_values][1][]', :multiple => 'multiple'} + assert_tag 'select', :attributes => {:name => 'issue[custom_field_values][1][]'}, + :child => {:tag => 'option', :attributes => {:value => 'MySQL', :selected => 'selected'}} + assert_tag 'select', :attributes => {:name => 'issue[custom_field_values][1][]'}, + :child => {:tag => 'option', :attributes => {:value => 'PostgreSQL', :selected => nil}} + assert_tag 'select', :attributes => {:name => 'issue[custom_field_values][1][]'}, + :child => {:tag => 'option', :attributes => {:value => 'Oracle', :selected => 'selected'}} + end + def test_update_edit_form @request.session[:user_id] = 2 xhr :put, :new, :project_id => 1, @@ -1979,6 +2145,27 @@ class IssuesControllerTest < ActionController::TestCase assert mail.body.include?("Searchable field changed from 125 to New custom value") end + def test_put_update_with_multi_custom_field_change + field = CustomField.find(1) + field.update_attribute :multiple, true + issue = Issue.find(1) + issue.custom_field_values = {1 => ['MySQL', 'Oracle']} + issue.save! + + @request.session[:user_id] = 2 + assert_difference('Journal.count') do + assert_difference('JournalDetail.count', 3) do + put :update, :id => 1, + :issue => { + :subject => 'Custom field change', + :custom_field_values => { '1' => ['', 'Oracle', 'PostgreSQL'] } + } + end + end + assert_redirected_to :action => 'show', :id => '1' + assert_equal ['Oracle', 'PostgreSQL'], Issue.find(1).custom_field_value(1).sort + end + def test_put_update_with_status_and_assignee_change issue = Issue.find(1) assert_equal 1, issue.status_id @@ -2283,6 +2470,23 @@ class IssuesControllerTest < ActionController::TestCase } end + def test_get_bulk_edit_with_multi_custom_field + field = CustomField.find(1) + field.update_attribute :multiple, true + + @request.session[:user_id] = 2 + get :bulk_edit, :ids => [1, 2] + assert_response :success + assert_template 'bulk_edit' + + assert_tag :select, + :attributes => {:name => "issue[custom_field_values][1][]"}, + :children => { + :only => {:tag => 'option'}, + :count => 3 + } + end + def test_bulk_update @request.session[:user_id] = 2 # update issues priority @@ -2463,6 +2667,24 @@ class IssuesControllerTest < ActionController::TestCase assert_equal '777', journal.details.first.value end + def test_bulk_update_multi_custom_field + field = CustomField.find(1) + field.update_attribute :multiple, true + + @request.session[:user_id] = 2 + post :bulk_update, :ids => [1, 2, 3], :notes => 'Bulk editing multi custom field', + :issue => {:priority_id => '', + :assigned_to_id => '', + :custom_field_values => {'1' => ['MySQL', 'Oracle']}} + + assert_response 302 + + assert_equal ['MySQL', 'Oracle'], Issue.find(1).custom_field_value(1).sort + assert_equal ['MySQL', 'Oracle'], Issue.find(3).custom_field_value(1).sort + # the custom field is not associated with the issue tracker + assert_nil Issue.find(2).custom_field_value(1) + end + def test_bulk_update_unassign assert_not_nil Issue.find(2).assigned_to @request.session[:user_id] = 2 diff --git a/test/integration/api_test/issues_test.rb b/test/integration/api_test/issues_test.rb index 0b7593e3f..5d40b3c85 100644 --- a/test/integration/api_test/issues_test.rb +++ b/test/integration/api_test/issues_test.rb @@ -258,6 +258,108 @@ class ApiTest::IssuesTest < ActionController::IntegrationTest end end + context "with multi custom fields" do + setup do + field = CustomField.find(1) + field.update_attribute :multiple, true + issue = Issue.find(3) + issue.custom_field_values = {1 => ['MySQL', 'Oracle']} + issue.save! + end + + context ".xml" do + should "display custom fields" do + get '/issues/3.xml' + assert_response :success + assert_tag :tag => 'issue', + :child => { + :tag => 'custom_fields', + :attributes => { :type => 'array' }, + :child => { + :tag => 'custom_field', + :attributes => { :id => '1'}, + :child => { + :tag => 'value', + :attributes => { :type => 'array' }, + :children => { :count => 2 } + } + } + } + + xml = Hash.from_xml(response.body) + custom_fields = xml['issue']['custom_fields'] + assert_kind_of Array, custom_fields + field = custom_fields.detect {|f| f['id'] == '1'} + assert_kind_of Hash, field + assert_equal ['MySQL', 'Oracle'], field['value'].sort + end + end + + context ".json" do + should "display custom fields" do + get '/issues/3.json' + assert_response :success + json = ActiveSupport::JSON.decode(response.body) + custom_fields = json['issue']['custom_fields'] + assert_kind_of Array, custom_fields + field = custom_fields.detect {|f| f['id'] == 1} + assert_kind_of Hash, field + assert_equal ['MySQL', 'Oracle'], field['value'].sort + end + end + end + + context "with empty value for multi custom field" do + setup do + field = CustomField.find(1) + field.update_attribute :multiple, true + issue = Issue.find(3) + issue.custom_field_values = {1 => ['']} + issue.save! + end + + context ".xml" do + should "display custom fields" do + get '/issues/3.xml' + assert_response :success + assert_tag :tag => 'issue', + :child => { + :tag => 'custom_fields', + :attributes => { :type => 'array' }, + :child => { + :tag => 'custom_field', + :attributes => { :id => '1'}, + :child => { + :tag => 'value', + :attributes => { :type => 'array' }, + :children => { :count => 0 } + } + } + } + + xml = Hash.from_xml(response.body) + custom_fields = xml['issue']['custom_fields'] + assert_kind_of Array, custom_fields + field = custom_fields.detect {|f| f['id'] == '1'} + assert_kind_of Hash, field + assert_equal [], field['value'] + end + end + + context ".json" do + should "display custom fields" do + get '/issues/3.json' + assert_response :success + json = ActiveSupport::JSON.decode(response.body) + custom_fields = json['issue']['custom_fields'] + assert_kind_of Array, custom_fields + field = custom_fields.detect {|f| f['id'] == 1} + assert_kind_of Hash, field + assert_equal [], field['value'].sort + end + end + end + context "with attachments" do context ".xml" do should "display attachments" do @@ -455,6 +557,24 @@ class ApiTest::IssuesTest < ActionController::IntegrationTest end end + context "PUT /issues/3.xml with multi custom fields" do + setup do + field = CustomField.find(1) + field.update_attribute :multiple, true + @parameters = {:issue => {:custom_fields => [{'id' => '1', 'value' => ['MySQL', 'PostgreSQL'] }, {'id' => '2', 'value' => '150'}]}} + end + + should "update custom fields" do + assert_no_difference('Issue.count') do + put '/issues/3.xml', @parameters, credentials('jsmith') + end + + issue = Issue.find(3) + assert_equal '150', issue.custom_value_for(2).value + assert_equal ['MySQL', 'PostgreSQL'], issue.custom_field_value(1) + end + end + context "PUT /issues/3.xml with project change" do setup do @parameters = {:issue => {:project_id => 2, :subject => 'Project changed'}} diff --git a/test/unit/custom_field_test.rb b/test/unit/custom_field_test.rb index 2d1183b72..6cef44ea9 100644 --- a/test/unit/custom_field_test.rb +++ b/test/unit/custom_field_test.rb @@ -164,4 +164,26 @@ class CustomFieldTest < ActiveSupport::TestCase assert f.valid_field_value?('5') assert !f.valid_field_value?('6abc') end + + def test_multi_field_validation + f = CustomField.new(:field_format => 'list', :multiple => 'true', :possible_values => ['value1', 'value2']) + + assert f.valid_field_value?(nil) + assert f.valid_field_value?('') + assert f.valid_field_value?([]) + assert f.valid_field_value?([nil]) + assert f.valid_field_value?(['']) + + assert f.valid_field_value?('value2') + assert !f.valid_field_value?('abc') + + assert f.valid_field_value?(['value2']) + assert !f.valid_field_value?(['abc']) + + assert f.valid_field_value?(['', 'value2']) + assert !f.valid_field_value?(['', 'abc']) + + assert f.valid_field_value?(['value1', 'value2']) + assert !f.valid_field_value?(['value1', 'abc']) + end end diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 2a5a14964..eda13d7a8 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -921,6 +921,36 @@ class IssueTest < ActiveSupport::TestCase end end + def test_journalized_multi_custom_field + field = IssueCustomField.create!(:name => 'filter', :field_format => 'list', :is_filter => true, :is_for_all => true, + :tracker_ids => [1], :possible_values => ['value1', 'value2', 'value3'], :multiple => true) + + issue = Issue.create!(:project_id => 1, :tracker_id => 1, :subject => 'Test', :author_id => 1) + + assert_difference 'Journal.count' do + assert_difference 'JournalDetail.count' do + issue.init_journal(User.first) + issue.custom_field_values = {field.id => ['value1']} + issue.save! + end + assert_difference 'JournalDetail.count' do + issue.init_journal(User.first) + issue.custom_field_values = {field.id => ['value1', 'value2']} + issue.save! + end + assert_difference 'JournalDetail.count', 2 do + issue.init_journal(User.first) + issue.custom_field_values = {field.id => ['value3', 'value2']} + issue.save! + end + assert_difference 'JournalDetail.count', 2 do + issue.init_journal(User.first) + issue.custom_field_values = {field.id => nil} + issue.save! + end + end + end + def test_description_eol_should_be_normalized i = Issue.new(:description => "CR \r LF \n CRLF \r\n") assert_equal "CR \r\n LF \r\n CRLF \r\n", i.description diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 840931d98..ca5bf9175 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -173,6 +173,44 @@ class QueryTest < ActiveSupport::TestCase assert_equal 2, issues.first.id end + def test_operator_is_on_multi_list_custom_field + f = IssueCustomField.create!(:name => 'filter', :field_format => 'list', :is_filter => true, :is_for_all => true, + :possible_values => ['value1', 'value2', 'value3'], :multiple => true) + CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => 'value1') + CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => 'value2') + CustomValue.create!(:custom_field => f, :customized => Issue.find(3), :value => 'value1') + + query = Query.new(:name => '_') + query.add_filter("cf_#{f.id}", '=', ['value1']) + issues = find_issues_with_query(query) + assert_equal [1, 3], issues.map(&:id).sort + + query = Query.new(:name => '_') + query.add_filter("cf_#{f.id}", '=', ['value2']) + issues = find_issues_with_query(query) + assert_equal [1], issues.map(&:id).sort + end + + def test_operator_is_not_on_multi_list_custom_field + f = IssueCustomField.create!(:name => 'filter', :field_format => 'list', :is_filter => true, :is_for_all => true, + :possible_values => ['value1', 'value2', 'value3'], :multiple => true) + CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => 'value1') + CustomValue.create!(:custom_field => f, :customized => Issue.find(1), :value => 'value2') + CustomValue.create!(:custom_field => f, :customized => Issue.find(3), :value => 'value1') + + query = Query.new(:name => '_') + query.add_filter("cf_#{f.id}", '!', ['value1']) + issues = find_issues_with_query(query) + assert !issues.map(&:id).include?(1) + assert !issues.map(&:id).include?(3) + + query = Query.new(:name => '_') + query.add_filter("cf_#{f.id}", '!', ['value2']) + issues = find_issues_with_query(query) + assert !issues.map(&:id).include?(1) + assert issues.map(&:id).include?(3) + end + def test_operator_greater_than query = Query.new(:project => Project.find(1), :name => '_') query.add_filter('done_ratio', '>=', ['40']) @@ -492,7 +530,18 @@ class QueryTest < ActiveSupport::TestCase def test_groupable_columns_should_include_custom_fields q = Query.new - assert q.groupable_columns.detect {|c| c.is_a? QueryCustomFieldColumn} + column = q.groupable_columns.detect {|c| c.name == :cf_1} + assert_not_nil column + assert_kind_of QueryCustomFieldColumn, column + end + + def test_groupable_columns_should_not_include_multi_custom_fields + field = CustomField.find(1) + field.update_attribute :multiple, true + + q = Query.new + column = q.groupable_columns.detect {|c| c.name == :cf_1} + assert_nil column end def test_grouped_with_valid_column @@ -527,6 +576,19 @@ class QueryTest < ActiveSupport::TestCase end end + def test_sortable_columns_should_include_custom_field + q = Query.new + assert q.sortable_columns['cf_1'] + end + + def test_sortable_columns_should_not_include_multi_custom_field + field = CustomField.find(1) + field.update_attribute :multiple, true + + q = Query.new + assert !q.sortable_columns['cf_1'] + end + def test_default_sort q = Query.new assert_equal [], q.sort_criteria diff --git a/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb b/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb index 943cc441c..88fe88b59 100644 --- a/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb +++ b/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb @@ -70,6 +70,12 @@ module Redmine key = custom_field_value.custom_field_id.to_s if values.has_key?(key) value = values[key] + if value.is_a?(Array) + value = value.reject(&:blank?).uniq + if value.empty? + value << '' + end + end custom_field_value.value = value end end @@ -81,9 +87,17 @@ module Redmine x = CustomFieldValue.new x.custom_field = field x.customized = self - cv = custom_values.detect { |v| v.custom_field == field } - cv ||= custom_values.build(:customized => self, :custom_field => field, :value => nil) - x.value = cv.value + if field.multiple? + values = custom_values.select { |v| v.custom_field == field } + if values.empty? + values << custom_values.build(:customized => self, :custom_field => field, :value => nil) + end + x.value = values.map(&:value) + else + cv = custom_values.detect { |v| v.custom_field == field } + cv ||= custom_values.build(:customized => self, :custom_field => field, :value => nil) + x.value = cv.value + end x end end @@ -115,10 +129,18 @@ module Redmine def save_custom_field_values target_custom_values = [] custom_field_values.each do |custom_field_value| - target = custom_values.detect {|cv| cv.custom_field == custom_field_value.custom_field} - target ||= custom_values.build(:customized => self, :custom_field => custom_field_value.custom_field) - target.value = custom_field_value.value - target_custom_values << target + if custom_field_value.value.is_a?(Array) + custom_field_value.value.each do |v| + target = custom_values.detect {|cv| cv.custom_field == custom_field_value.custom_field && cv.value == v} + target ||= custom_values.build(:customized => self, :custom_field => custom_field_value.custom_field, :value => v) + target_custom_values << target + end + else + target = custom_values.detect {|cv| cv.custom_field == custom_field_value.custom_field} + target ||= custom_values.build(:customized => self, :custom_field => custom_field_value.custom_field) + target.value = custom_field_value.value + target_custom_values << target + end end self.custom_values = target_custom_values custom_values.each(&:save)