From ce6cf66f6c3af26383cd25ed012d908be4b40bae Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Fri, 27 Jun 2008 20:13:56 +0000 Subject: [PATCH] Custom fields refactoring: most of code moved from controllers to models (using new module ActsAsCustomizable). git-svn-id: http://redmine.rubyforge.org/svn/trunk@1592 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/account_controller.rb | 5 -- app/controllers/issues_controller.rb | 24 ++--- app/controllers/projects_controller.rb | 14 +-- app/controllers/timelog_controller.rb | 2 +- app/controllers/users_controller.rb | 11 +-- app/helpers/custom_fields_helper.rb | 27 +++--- app/helpers/issues_helper.rb | 2 +- app/models/issue.rb | 14 ++- app/models/project.rb | 13 +-- app/models/query.rb | 4 +- app/models/user.rb | 8 +- app/views/account/register.rhtml | 4 +- app/views/issues/_form_custom_fields.rhtml | 13 +-- app/views/issues/show.rhtml | 6 +- app/views/projects/_form.rhtml | 12 +-- app/views/projects/show.rhtml | 2 +- app/views/users/_form.rhtml | 6 +- test/fixtures/custom_fields.yml | 11 ++- test/functional/issues_controller_test.rb | 87 +++++++++++++++++-- test/functional/projects_controller_test.rb | 2 +- test/integration/admin_test.rb | 5 +- test/unit/issue_test.rb | 78 ++++++++++++++++- vendor/plugins/acts_as_customizable/init.rb | 2 + .../lib/acts_as_customizable.rb | 82 +++++++++++++++++ 24 files changed, 321 insertions(+), 113 deletions(-) create mode 100644 vendor/plugins/acts_as_customizable/init.rb create mode 100644 vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb diff --git a/app/controllers/account_controller.rb b/app/controllers/account_controller.rb index 3fbbc891..a9b8a1b8 100644 --- a/app/controllers/account_controller.rb +++ b/app/controllers/account_controller.rb @@ -110,17 +110,12 @@ class AccountController < ApplicationController redirect_to(home_url) && return unless Setting.self_registration? if request.get? @user = User.new(:language => Setting.default_language) - @custom_values = UserCustomField.find(:all).collect { |x| CustomValue.new(:custom_field => x, :customized => @user) } else @user = User.new(params[:user]) @user.admin = false @user.login = params[:user][:login] @user.status = User::STATUS_REGISTERED @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] - @custom_values = UserCustomField.find(:all).collect { |x| CustomValue.new(:custom_field => x, - :customized => @user, - :value => (params["custom_fields"] ? params["custom_fields"][x.id.to_s] : nil)) } - @user.custom_values = @custom_values case Setting.self_registration when '1' # Email activation diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 69c8e793..49b44323 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -94,7 +94,6 @@ class IssuesController < ApplicationController end def show - @custom_values = @project.custom_fields_for_issues(@issue.tracker).collect { |x| @issue.custom_values.find_by_custom_field_id(x.id) || CustomValue.new(:custom_field => x, :customized => @issue) } @journals = @issue.journals.find(:all, :include => [:user, :details], :order => "#{Journal.table_name}.created_on ASC") @journals.each_with_index {|j,i| j.indice = i+1} @journals.reverse! if User.current.wants_comments_in_reverse_order? @@ -113,15 +112,17 @@ class IssuesController < ApplicationController # Add a new issue # The new issue will be created from an existing one if copy_from parameter is given def new - @issue = params[:copy_from] ? Issue.new.copy_from(params[:copy_from]) : Issue.new(params[:issue]) + @issue = Issue.new + @issue.copy_from(params[:copy_from]) if params[:copy_from] @issue.project = @project - @issue.author = User.current @issue.tracker ||= @project.trackers.find(params[:tracker_id] ? params[:tracker_id] : :first) if @issue.tracker.nil? flash.now[:error] = 'No tracker is associated to this project. Please check the Project settings.' render :nothing => true, :layout => true return end + @issue.attributes = params[:issue] + @issue.author = User.current default_status = IssueStatus.default unless default_status @@ -134,17 +135,10 @@ class IssuesController < ApplicationController if request.get? || request.xhr? @issue.start_date ||= Date.today - @custom_values = @issue.custom_values.empty? ? - @project.custom_fields_for_issues(@issue.tracker).collect { |x| CustomValue.new(:custom_field => x, :customized => @issue) } : - @issue.custom_values else requested_status = IssueStatus.find_by_id(params[:issue][:status_id]) # Check that the user is allowed to apply the requested status @issue.status = (@allowed_statuses.include? requested_status) ? requested_status : default_status - @custom_values = @project.custom_fields_for_issues(@issue.tracker).collect { |x| CustomValue.new(:custom_field => x, - :customized => @issue, - :value => (params[:custom_fields] ? params[:custom_fields][x.id.to_s] : nil)) } - @issue.custom_values = @custom_values if @issue.save attach_files(@issue, params[:attachments]) flash[:notice] = l(:notice_successful_create) @@ -165,7 +159,6 @@ class IssuesController < ApplicationController @allowed_statuses = @issue.new_statuses_allowed_to(User.current) @activities = Enumeration::get_values('ACTI') @priorities = Enumeration::get_values('IPRI') - @custom_values = [] @edit_allowed = User.current.allowed_to?(:edit_issues, @project) @notes = params[:notes] @@ -178,14 +171,7 @@ class IssuesController < ApplicationController @issue.attributes = attrs end - if request.get? - @custom_values = @project.custom_fields_for_issues(@issue.tracker).collect { |x| @issue.custom_values.find_by_custom_field_id(x.id) || CustomValue.new(:custom_field => x, :customized => @issue) } - else - # Update custom fields if user has :edit permission - if @edit_allowed && params[:custom_fields] - @custom_values = @project.custom_fields_for_issues(@issue.tracker).collect { |x| CustomValue.new(:custom_field => x, :customized => @issue, :value => params["custom_fields"][x.id.to_s]) } - @issue.custom_values = @custom_values - end + if request.post? @time_entry = TimeEntry.new(:project => @project, :issue => @issue, :user => User.current, :spent_on => Date.today) @time_entry.attributes = params[:time_entry] attachments = attach_files(@issue, params[:attachments]) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index c9a55088..a37ae09a 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -63,21 +63,17 @@ class ProjectsController < ApplicationController # Add a new project def add - @custom_fields = IssueCustomField.find(:all, :order => "#{CustomField.table_name}.position") + @issue_custom_fields = IssueCustomField.find(:all, :order => "#{CustomField.table_name}.position") @trackers = Tracker.all @root_projects = Project.find(:all, :conditions => "parent_id IS NULL AND status = #{Project::STATUS_ACTIVE}", :order => 'name') @project = Project.new(params[:project]) if request.get? - @custom_values = ProjectCustomField.find(:all, :order => "#{CustomField.table_name}.position").collect { |x| CustomValue.new(:custom_field => x, :customized => @project) } @project.trackers = Tracker.all @project.is_public = Setting.default_projects_public? @project.enabled_module_names = Redmine::AccessControl.available_project_modules else - @project.custom_fields = CustomField.find(params[:custom_field_ids]) if params[:custom_field_ids] - @custom_values = ProjectCustomField.find(:all, :order => "#{CustomField.table_name}.position").collect { |x| CustomValue.new(:custom_field => x, :customized => @project, :value => (params[:custom_fields] ? params["custom_fields"][x.id.to_s] : nil)) } - @project.custom_values = @custom_values @project.enabled_module_names = params[:enabled_modules] if @project.save flash[:notice] = l(:notice_successful_create) @@ -88,7 +84,6 @@ class ProjectsController < ApplicationController # Show @project def show - @custom_values = @project.custom_values.find(:all, :include => :custom_field, :order => "#{CustomField.table_name}.position") @members_by_role = @project.members.find(:all, :include => [:user, :role], :order => 'position').group_by {|m| m.role} @subprojects = @project.children.find(:all, :conditions => Project.visible_by(User.current)) @news = @project.news.find(:all, :limit => 5, :include => [ :author, :project ], :order => "#{News.table_name}.created_on DESC") @@ -115,11 +110,10 @@ class ProjectsController < ApplicationController @root_projects = Project.find(:all, :conditions => ["parent_id IS NULL AND status = #{Project::STATUS_ACTIVE} AND id <> ?", @project.id], :order => 'name') - @custom_fields = IssueCustomField.find(:all) + @issue_custom_fields = IssueCustomField.find(:all, :order => "#{CustomField.table_name}.position") @issue_category ||= IssueCategory.new @member ||= @project.members.new @trackers = Tracker.all - @custom_values ||= ProjectCustomField.find(:all, :order => "#{CustomField.table_name}.position").collect { |x| @project.custom_values.find_by_custom_field_id(x.id) || CustomValue.new(:custom_field => x) } @repository ||= @project.repository @wiki ||= @project.wiki end @@ -127,10 +121,6 @@ class ProjectsController < ApplicationController # Edit @project def edit if request.post? - if params[:custom_fields] - @custom_values = ProjectCustomField.find(:all, :order => "#{CustomField.table_name}.position").collect { |x| CustomValue.new(:custom_field => x, :customized => @project, :value => params["custom_fields"][x.id.to_s]) } - @project.custom_values = @custom_values - end @project.attributes = params[:project] if @project.save flash[:notice] = l(:notice_successful_update) diff --git a/app/controllers/timelog_controller.rb b/app/controllers/timelog_controller.rb index 2e257c5a..cf1c844d 100644 --- a/app/controllers/timelog_controller.rb +++ b/app/controllers/timelog_controller.rb @@ -54,7 +54,7 @@ class TimelogController < ApplicationController } # Add list and boolean custom fields as available criterias - @project.all_custom_fields.select {|cf| %w(list bool).include? cf.field_format }.each do |cf| + @project.all_issue_custom_fields.select {|cf| %w(list bool).include? cf.field_format }.each do |cf| @available_criterias["cf_#{cf.id}"] = {:sql => "(SELECT c.value FROM custom_values c WHERE c.custom_field_id = #{cf.id} AND c.customized_type = 'Issue' AND c.customized_id = issues.id)", :format => cf.field_format, :label => cf.name} diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index c3770966..eb8aa7ba 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -52,14 +52,11 @@ class UsersController < ApplicationController def add if request.get? @user = User.new(:language => Setting.default_language) - @custom_values = UserCustomField.find(:all, :order => "#{CustomField.table_name}.position").collect { |x| CustomValue.new(:custom_field => x, :customized => @user) } else @user = User.new(params[:user]) @user.admin = params[:user][:admin] || false @user.login = params[:user][:login] @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] unless @user.auth_source_id - @custom_values = UserCustomField.find(:all, :order => "#{CustomField.table_name}.position").collect { |x| CustomValue.new(:custom_field => x, :customized => @user, :value => (params[:custom_fields] ? params["custom_fields"][x.id.to_s] : nil)) } - @user.custom_values = @custom_values if @user.save Mailer.deliver_account_information(@user, params[:password]) if params[:send_information] flash[:notice] = l(:notice_successful_create) @@ -71,16 +68,10 @@ class UsersController < ApplicationController def edit @user = User.find(params[:id]) - if request.get? - @custom_values = UserCustomField.find(:all, :order => "#{CustomField.table_name}.position").collect { |x| @user.custom_values.find_by_custom_field_id(x.id) || CustomValue.new(:custom_field => x) } - else + if request.post? @user.admin = params[:user][:admin] if params[:user][:admin] @user.login = params[:user][:login] if params[:user][:login] @user.password, @user.password_confirmation = params[:password], params[:password_confirmation] unless params[:password].nil? or params[:password].empty? or @user.auth_source_id - if params[:custom_fields] - @custom_values = UserCustomField.find(:all, :order => "#{CustomField.table_name}.position").collect { |x| CustomValue.new(:custom_field => x, :customized => @user, :value => params["custom_fields"][x.id.to_s]) } - @user.custom_values = @custom_values - end if @user.update_attributes(params[:user]) flash[:notice] = l(:notice_successful_update) # Give a string to redirect_to otherwise it would use status param as the response code diff --git a/app/helpers/custom_fields_helper.rb b/app/helpers/custom_fields_helper.rb index 61c8d6b3..540c6b4a 100644 --- a/app/helpers/custom_fields_helper.rb +++ b/app/helpers/custom_fields_helper.rb @@ -25,37 +25,40 @@ module CustomFieldsHelper end # Return custom field html tag corresponding to its format - def custom_field_tag(custom_value) + def custom_field_tag(name, custom_value) custom_field = custom_value.custom_field - field_name = "custom_fields[#{custom_field.id}]" - field_id = "custom_fields_#{custom_field.id}" + field_name = "#{name}[custom_field_values][#{custom_field.id}]" + field_id = "#{name}_custom_field_values_#{custom_field.id}" case custom_field.field_format when "date" - text_field('custom_value', 'value', :name => field_name, :id => field_id, :size => 10) + + text_field_tag(field_name, custom_value.value, :id => field_id, :size => 10) + calendar_for(field_id) when "text" - text_area 'custom_value', 'value', :name => field_name, :id => field_id, :rows => 3, :style => 'width:99%' + text_area_tag(field_name, custom_value.value, :id => field_id, :rows => 3, :style => 'width:90%') when "bool" - check_box 'custom_value', 'value', :name => field_name, :id => field_id + check_box_tag(field_name, custom_value.value, :id => field_id) when "list" - select 'custom_value', 'value', custom_field.possible_values, { :include_blank => true }, :name => field_name, :id => field_id + blank_option = custom_field.is_required? ? + (custom_field.default_value.blank? ? "" : '') : + '' + select_tag(field_name, blank_option + options_for_select(custom_field.possible_values, custom_value.value), :id => field_id) else - text_field 'custom_value', 'value', :name => field_name, :id => field_id + text_field_tag(field_name, custom_value.value, :id => field_id) end end # Return custom field label tag - def custom_field_label_tag(custom_value) + def custom_field_label_tag(name, custom_value) content_tag "label", custom_value.custom_field.name + (custom_value.custom_field.is_required? ? " *" : ""), - :for => "custom_fields_#{custom_value.custom_field.id}", + :for => "#{name}_custom_field_values_#{custom_value.custom_field.id}", :class => (custom_value.errors.empty? ? nil : "error" ) end # Return custom field tag with its label tag - def custom_field_tag_with_label(custom_value) - custom_field_label_tag(custom_value) + custom_field_tag(custom_value) + def custom_field_tag_with_label(name, custom_value) + custom_field_label_tag(name, custom_value) + custom_field_tag(name, custom_value) end # Return a string used to display a custom value diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index f42002ec..40369717 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -149,7 +149,7 @@ module IssuesHelper ] # Export project custom fields if project is given # otherwise export custom fields marked as "For all projects" - custom_fields = project.nil? ? IssueCustomField.for_all : project.all_custom_fields + custom_fields = project.nil? ? IssueCustomField.for_all : project.all_issue_custom_fields custom_fields.each {|f| headers << f.name} # Description in the last column headers << l(:field_description) diff --git a/app/models/issue.rb b/app/models/issue.rb index d83b2ab0..326e234b 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -28,13 +28,12 @@ class Issue < ActiveRecord::Base has_many :journals, :as => :journalized, :dependent => :destroy has_many :attachments, :as => :container, :dependent => :destroy has_many :time_entries, :dependent => :delete_all - has_many :custom_values, :dependent => :delete_all, :as => :customized - has_many :custom_fields, :through => :custom_values has_and_belongs_to_many :changesets, :order => "revision ASC" has_many :relations_from, :class_name => 'IssueRelation', :foreign_key => 'issue_from_id', :dependent => :delete_all has_many :relations_to, :class_name => 'IssueRelation', :foreign_key => 'issue_to_id', :dependent => :delete_all + acts_as_customizable acts_as_watchable acts_as_searchable :columns => ['subject', "#{table_name}.description"], :include => :project, :with => {:journal => :issue} acts_as_event :title => Proc.new {|o| "#{o.tracker.name} ##{o.id}: #{o.subject}"}, @@ -44,7 +43,6 @@ class Issue < ActiveRecord::Base validates_length_of :subject, :maximum => 255 validates_inclusion_of :done_ratio, :in => 0..100 validates_numericality_of :estimated_hours, :allow_nil => true - validates_associated :custom_values, :on => :update def after_initialize if new_record? @@ -54,6 +52,11 @@ class Issue < ActiveRecord::Base end end + # Overrides Redmine::Acts::Customizable::InstanceMethods#available_custom_fields + def available_custom_fields + (project && tracker) ? project.all_issue_custom_fields.select {|c| tracker.custom_fields.include? c } : [] + end + def copy_from(arg) issue = arg.is_a?(Issue) ? arg : Issue.find(arg) self.attributes = issue.attributes.dup @@ -168,11 +171,6 @@ class Issue < ActiveRecord::Base end end - def custom_value_for(custom_field) - self.custom_values.each {|v| return v if v.custom_field_id == custom_field.id } - return nil - end - def init_journal(user, notes = "") @current_journal ||= Journal.new(:journalized => self, :user => user, :notes => notes) @issue_before_change = self.clone diff --git a/app/models/project.rb b/app/models/project.rb index f05ccb2a..a5ba246b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -22,7 +22,6 @@ class Project < ActiveRecord::Base has_many :members, :include => :user, :conditions => "#{User.table_name}.status=#{User::STATUS_ACTIVE}" has_many :users, :through => :members - has_many :custom_values, :dependent => :delete_all, :as => :customized has_many :enabled_modules, :dependent => :delete_all has_and_belongs_to_many :trackers, :order => "#{Tracker.table_name}.position" has_many :issues, :dependent => :destroy, :order => "#{Issue.table_name}.created_on DESC", :include => [:status, :tracker] @@ -38,7 +37,7 @@ class Project < ActiveRecord::Base has_many :changesets, :through => :repository has_one :wiki, :dependent => :destroy # Custom field for the project issues - has_and_belongs_to_many :custom_fields, + has_and_belongs_to_many :issue_custom_fields, :class_name => 'IssueCustomField', :order => "#{CustomField.table_name}.position", :join_table => "#{table_name_prefix}custom_fields_projects#{table_name_suffix}", @@ -46,6 +45,7 @@ class Project < ActiveRecord::Base acts_as_tree :order => "name", :counter_cache => true + acts_as_customizable acts_as_searchable :columns => ['name', 'description'], :project_key => 'id', :permission => nil acts_as_event :title => Proc.new {|o| "#{l(:label_project)}: #{o.name}"}, :url => Proc.new {|o| {:controller => 'projects', :action => 'show', :id => o.id}} @@ -54,7 +54,6 @@ class Project < ActiveRecord::Base validates_presence_of :name, :identifier validates_uniqueness_of :name, :identifier - validates_associated :custom_values, :on => :update validates_associated :repository, :wiki validates_length_of :name, :maximum => 30 validates_length_of :homepage, :maximum => 255 @@ -195,12 +194,8 @@ class Project < ActiveRecord::Base # Returns an array of all custom fields enabled for project issues # (explictly associated custom fields and custom fields enabled for all projects) - def custom_fields_for_issues(tracker) - all_custom_fields.select {|c| tracker.custom_fields.include? c } - end - - def all_custom_fields - @all_custom_fields ||= (IssueCustomField.for_all + custom_fields).uniq + def all_issue_custom_fields + @all_issue_custom_fields ||= (IssueCustomField.for_all + issue_custom_fields).uniq end def project diff --git a/app/models/query.rb b/app/models/query.rb index 4c72e23f..27ab882c 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -176,7 +176,7 @@ class Query < ActiveRecord::Base unless @project.active_children.empty? @available_filters["subproject_id"] = { :type => :list_subprojects, :order => 13, :values => @project.active_children.collect{|s| [s.name, s.id.to_s] } } end - add_custom_fields_filters(@project.all_custom_fields) + add_custom_fields_filters(@project.all_issue_custom_fields) else # global filters for cross project issue list add_custom_fields_filters(IssueCustomField.find(:all, :conditions => {:is_filter => true, :is_for_all => true})) @@ -226,7 +226,7 @@ class Query < ActiveRecord::Base return @available_columns if @available_columns @available_columns = Query.available_columns @available_columns += (project ? - project.all_custom_fields : + project.all_issue_custom_fields : IssueCustomField.find(:all, :conditions => {:is_for_all => true}) ).collect {|cf| QueryCustomFieldColumn.new(cf) } end diff --git a/app/models/user.rb b/app/models/user.rb index 5568027d..a34b9686 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -37,12 +37,13 @@ class User < ActiveRecord::Base has_many :memberships, :class_name => 'Member', :include => [ :project, :role ], :conditions => "#{Project.table_name}.status=#{Project::STATUS_ACTIVE}", :order => "#{Project.table_name}.name", :dependent => :delete_all has_many :projects, :through => :memberships - has_many :custom_values, :dependent => :delete_all, :as => :customized has_many :issue_categories, :foreign_key => 'assigned_to_id', :dependent => :nullify has_one :preference, :dependent => :destroy, :class_name => 'UserPreference' has_one :rss_token, :dependent => :destroy, :class_name => 'Token', :conditions => "action='feeds'" belongs_to :auth_source + acts_as_customizable + attr_accessor :password, :password_confirmation attr_accessor :last_before_login_on # Prevents unauthorized assignments @@ -60,7 +61,6 @@ class User < ActiveRecord::Base validates_length_of :mail, :maximum => 60, :allow_nil => true validates_length_of :password, :minimum => 4, :allow_nil => true validates_confirmation_of :password, :allow_nil => true - validates_associated :custom_values, :on => :update def before_create self.mail_notification = false @@ -280,6 +280,10 @@ class AnonymousUser < User errors.add_to_base 'An anonymous user already exists.' if AnonymousUser.find(:first) end + def available_custom_fields + [] + end + # Overrides a few properties def logged?; false end def admin; false end diff --git a/app/views/account/register.rhtml b/app/views/account/register.rhtml index 7cf4b6da..4e2b5adf 100644 --- a/app/views/account/register.rhtml +++ b/app/views/account/register.rhtml @@ -27,8 +27,8 @@

<%= select("user", "language", lang_options_for_select) %>

-<% for @custom_value in @custom_values %> -

<%= custom_field_tag_with_label @custom_value %>

+<% @user.custom_field_values.each do |value| %> +

<%= custom_field_tag_with_label :user, value %>

<% end %> diff --git a/app/views/issues/_form_custom_fields.rhtml b/app/views/issues/_form_custom_fields.rhtml index 1268bb1f..ebd4c321 100644 --- a/app/views/issues/_form_custom_fields.rhtml +++ b/app/views/issues/_form_custom_fields.rhtml @@ -1,11 +1,12 @@
<% i = 1 %> -<% for @custom_value in values %> -

<%= custom_field_tag_with_label @custom_value %>

- <% if i == values.size / 2 %> +<% split_on = @issue.custom_field_values.size / 2 %> +<% @issue.custom_field_values.each do |value| %> +

<%= custom_field_tag_with_label :issue, value %>

+<% if i == split_on -%>
- <% end %> - <% i += 1 %> -<% end %> +<% end -%> +<% i += 1 -%> +<% end -%>
diff --git a/app/views/issues/show.rhtml b/app/views/issues/show.rhtml index 57fbd05c..a3b26be1 100644 --- a/app/views/issues/show.rhtml +++ b/app/views/issues/show.rhtml @@ -43,9 +43,9 @@ <% end %> -<% n = 0 -for custom_value in @custom_values %> - <%= custom_value.custom_field.name %>:<%= simple_format(h(show_value(custom_value))) %> +<% n = 0 -%> +<% @issue.custom_values.each do |value| -%> + <%=h value.custom_field.name %>:<%= simple_format(h(show_value(value))) %> <% n = n + 1 if (n > 1) n = 0 %> diff --git a/app/views/projects/_form.rhtml b/app/views/projects/_form.rhtml index 774e7397..11f7e393 100644 --- a/app/views/projects/_form.rhtml +++ b/app/views/projects/_form.rhtml @@ -17,8 +17,8 @@

<%= f.check_box :is_public %>

<%= wikitoolbar_for 'project_description' %> -<% for @custom_value in @custom_values %> -

<%= custom_field_tag_with_label @custom_value %>

+<% @project.custom_field_values.each do |value| %> +

<%= custom_field_tag_with_label :project, value %>

<% end %> @@ -34,15 +34,15 @@ <% end %> -<% unless @custom_fields.empty? %> +<% unless @issue_custom_fields.empty? %>
<%=l(:label_custom_field_plural)%> -<% for custom_field in @custom_fields %> +<% @issue_custom_fields.each do |custom_field| %> <% end %> -<%= hidden_field_tag 'project[custom_field_ids][]', '' %> +<%= hidden_field_tag 'project[issue_custom_field_ids][]', '' %>
<% end %> diff --git a/app/views/projects/show.rhtml b/app/views/projects/show.rhtml index 62b91193..6c82c80b 100644 --- a/app/views/projects/show.rhtml +++ b/app/views/projects/show.rhtml @@ -10,7 +10,7 @@ <% if @project.parent %>
  • <%=l(:field_parent)%>: <%= link_to h(@project.parent.name), :controller => 'projects', :action => 'show', :id => @project.parent %>
    • <% end %> - <% for custom_value in @custom_values %> + <% @project.custom_values.each do |custom_value| %> <% if !custom_value.value.empty? %>
  • <%= custom_value.custom_field.name%>: <%=h show_value(custom_value) %>
    • <% end %> diff --git a/app/views/users/_form.rhtml b/app/views/users/_form.rhtml index 09a79846..799ebde4 100644 --- a/app/views/users/_form.rhtml +++ b/app/views/users/_form.rhtml @@ -8,9 +8,9 @@

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

    <%= f.select :language, lang_options_for_select %>

    -<% for @custom_value in @custom_values %> -

    <%= custom_field_tag_with_label @custom_value %>

    -<% end if @custom_values%> +<% @user.custom_field_values.each do |value| %> +

    <%= custom_field_tag_with_label :user, value %>

    +<% end %>

    <%= f.check_box :admin, :disabled => (@user == User.current) %>

    diff --git a/test/fixtures/custom_fields.yml b/test/fixtures/custom_fields.yml index 9d88bc6f..1005edae 100644 --- a/test/fixtures/custom_fields.yml +++ b/test/fixtures/custom_fields.yml @@ -7,7 +7,10 @@ custom_fields_001: is_filter: true type: IssueCustomField max_length: 0 - possible_values: MySQL|PostgreSQL|Oracle + possible_values: + - MySQL + - PostgreSQL + - Oracle id: 1 is_required: false field_format: list @@ -33,7 +36,11 @@ custom_fields_003: is_filter: true type: ProjectCustomField max_length: 0 - possible_values: Stable|Beta|Alpha|Planning + possible_values: + - Stable + - Beta + - Alpha + - Planning id: 3 is_required: true field_format: list diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 32d2a7f4..ac1f4f83 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -170,7 +170,7 @@ class IssuesControllerTest < Test::Unit::TestCase assert_response :success assert_template 'new' - assert_tag :tag => 'input', :attributes => { :name => 'custom_fields[2]', + assert_tag :tag => 'input', :attributes => { :name => 'issue[custom_field_values][2]', :value => 'Default string' } end @@ -203,8 +203,8 @@ class IssuesControllerTest < Test::Unit::TestCase :subject => 'This is the test_new issue', :description => 'This is the description', :priority_id => 5, - :estimated_hours => ''}, - :custom_fields => {'2' => 'Value for field 2'} + :estimated_hours => '', + :custom_field_values => {'2' => 'Value for field 2'}} assert_redirected_to 'issues/show' issue = Issue.find_by_subject('This is the test_new issue') @@ -226,6 +226,50 @@ class IssuesControllerTest < Test::Unit::TestCase assert_redirected_to 'issues/show' end + def test_post_new_with_required_custom_field_and_without_custom_fields_param + field = IssueCustomField.find_by_name('Database') + field.update_attribute(:is_required, true) + + @request.session[:user_id] = 2 + post :new, :project_id => 1, + :issue => {:tracker_id => 1, + :subject => 'This is the test_new issue', + :description => 'This is the description', + :priority_id => 5} + assert_response :success + assert_template 'new' + issue = assigns(:issue) + assert_not_nil issue + assert_equal 'activerecord_error_invalid', issue.errors.on(:custom_values) + end + + def test_post_should_preserve_fields_values_on_validation_failure + @request.session[:user_id] = 2 + post :new, :project_id => 1, + :issue => {:tracker_id => 1, + :subject => 'This is the test_new issue', + # empty description + :description => '', + :priority_id => 6, + :custom_field_values => {'1' => 'Oracle', '2' => 'Value for field 2'}} + assert_response :success + assert_template 'new' + + assert_tag :input, :attributes => { :name => 'issue[subject]', + :value => 'This is the test_new issue' } + assert_tag :select, :attributes => { :name => 'issue[priority_id]' }, + :child => { :tag => 'option', :attributes => { :selected => 'selected', + :value => '6' }, + :content => 'High' } + # Custom fields + assert_tag :select, :attributes => { :name => 'issue[custom_field_values][1]' }, + :child => { :tag => 'option', :attributes => { :selected => 'selected', + :value => 'Oracle' }, + :content => 'Oracle' } + assert_tag :input, :attributes => { :name => 'issue[custom_field_values][2]', + :value => 'Value for field 2'} + end + def test_copy_issue @request.session[:user_id] = 2 get :new, :project_id => 1, :copy_from => 1 @@ -280,18 +324,28 @@ class IssuesControllerTest < Test::Unit::TestCase assert_select_rjs :show, "update" end - def test_post_edit + def test_post_edit_without_custom_fields_param @request.session[:user_id] = 2 ActionMailer::Base.deliveries.clear issue = Issue.find(1) + assert_equal '125', issue.custom_value_for(2).value old_subject = issue.subject new_subject = 'Subject modified by IssuesControllerTest#test_post_edit' - post :edit, :id => 1, :issue => {:subject => new_subject} + assert_difference('Journal.count') do + assert_difference('JournalDetail.count', 2) do + post :edit, :id => 1, :issue => {:subject => new_subject, + :priority_id => '6', + :category_id => '1' # no change + } + end + end assert_redirected_to 'issues/show/1' issue.reload assert_equal new_subject, issue.subject + # Make sure custom fields were not cleared + assert_equal '125', issue.custom_value_for(2).value mail = ActionMailer::Base.deliveries.last assert_kind_of TMail::Mail, mail @@ -299,6 +353,29 @@ class IssuesControllerTest < Test::Unit::TestCase assert mail.body.include?("Subject changed from #{old_subject} to #{new_subject}") end + def test_post_edit_with_custom_field_change + @request.session[:user_id] = 2 + issue = Issue.find(1) + assert_equal '125', issue.custom_value_for(2).value + + assert_difference('Journal.count') do + assert_difference('JournalDetail.count', 3) do + post :edit, :id => 1, :issue => {:subject => 'Custom field change', + :priority_id => '6', + :category_id => '1', # no change + :custom_field_values => { '2' => 'New custom value' } + } + end + end + assert_redirected_to 'issues/show/1' + issue.reload + assert_equal 'New custom value', issue.custom_value_for(2).value + + mail = ActionMailer::Base.deliveries.last + assert_kind_of TMail::Mail, mail + assert mail.body.include?("Searchable field changed from 125 to New custom value") + end + def test_post_edit_with_status_and_assignee_change issue = Issue.find(1) assert_equal 1, issue.status_id diff --git a/test/functional/projects_controller_test.rb b/test/functional/projects_controller_test.rb index 5b7c29b1..88c0319b 100644 --- a/test/functional/projects_controller_test.rb +++ b/test/functional/projects_controller_test.rb @@ -83,7 +83,7 @@ class ProjectsControllerTest < Test::Unit::TestCase def test_edit @request.session[:user_id] = 2 # manager post :edit, :id => 1, :project => {:name => 'Test changed name', - :custom_field_ids => ['']} + :issue_custom_field_ids => ['']} assert_redirected_to 'projects/settings/ecookbook' project = Project.find(1) assert_equal 'Test changed name', project.name diff --git a/test/integration/admin_test.rb b/test/integration/admin_test.rb index a424247c..6e385873 100644 --- a/test/integration/admin_test.rb +++ b/test/integration/admin_test.rb @@ -48,8 +48,9 @@ class AdminTest < ActionController::IntegrationTest post "projects/add", :project => { :name => "blog", :description => "weblog", :identifier => "blog", - :is_public => 1 }, - 'custom_fields[3]' => 'Beta' + :is_public => 1, + :custom_field_values => { '3' => 'Beta' } + } assert_redirected_to "admin/projects" assert_equal 'Successful creation.', flash[:notice] diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 999c4480..0d98f89d 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -18,7 +18,13 @@ require File.dirname(__FILE__) + '/../test_helper' class IssueTest < Test::Unit::TestCase - fixtures :projects, :users, :members, :trackers, :projects_trackers, :issue_statuses, :issue_categories, :enumerations, :issues, :custom_fields, :custom_values, :time_entries + fixtures :projects, :users, :members, + :trackers, :projects_trackers, + :issue_statuses, :issue_categories, + :enumerations, + :issues, + :custom_fields, :custom_fields_projects, :custom_fields_trackers, :custom_values, + :time_entries def test_create issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => Enumeration.get_values('IPRI').first, :subject => 'test_create', :description => 'IssueTest#test_create', :estimated_hours => '1:30') @@ -27,6 +33,76 @@ class IssueTest < Test::Unit::TestCase assert_equal 1.5, issue.estimated_hours end + def test_create_with_required_custom_field + field = IssueCustomField.find_by_name('Database') + field.update_attribute(:is_required, true) + + issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 1, :status_id => 1, :subject => 'test_create', :description => 'IssueTest#test_create_with_required_custom_field') + assert issue.available_custom_fields.include?(field) + # No value for the custom field + assert !issue.save + assert_equal 'activerecord_error_invalid', issue.errors.on(:custom_values) + # Blank value + issue.custom_field_values = { field.id => '' } + assert !issue.save + assert_equal 'activerecord_error_invalid', issue.errors.on(:custom_values) + # Invalid value + issue.custom_field_values = { field.id => 'SQLServer' } + assert !issue.save + assert_equal 'activerecord_error_invalid', issue.errors.on(:custom_values) + # Valid value + issue.custom_field_values = { field.id => 'PostgreSQL' } + assert issue.save + issue.reload + assert_equal 'PostgreSQL', issue.custom_value_for(field).value + end + + def test_update_issue_with_required_custom_field + field = IssueCustomField.find_by_name('Database') + field.update_attribute(:is_required, true) + + issue = Issue.find(1) + assert_nil issue.custom_value_for(field) + assert issue.available_custom_fields.include?(field) + # No change to custom values, issue can be saved + assert issue.save + # Blank value + issue.custom_field_values = { field.id => '' } + assert !issue.save + # Valid value + issue.custom_field_values = { field.id => 'PostgreSQL' } + assert issue.save + issue.reload + assert_equal 'PostgreSQL', issue.custom_value_for(field).value + end + + def test_should_not_update_attributes_if_custom_fields_validation_fails + issue = Issue.find(1) + field = IssueCustomField.find_by_name('Database') + assert issue.available_custom_fields.include?(field) + + issue.custom_field_values = { field.id => 'Invalid' } + issue.subject = 'Should be not be saved' + assert !issue.save + + issue.reload + assert_equal "Can't print recipes", issue.subject + end + + def test_should_not_recreate_custom_values_objects_on_update + field = IssueCustomField.find_by_name('Database') + + issue = Issue.find(1) + issue.custom_field_values = { field.id => 'PostgreSQL' } + assert issue.save + custom_value = issue.custom_value_for(field) + issue.reload + issue.custom_field_values = { field.id => 'MySQL' } + assert issue.save + issue.reload + assert_equal custom_value.id, issue.custom_value_for(field).id + end + def test_category_based_assignment issue = Issue.create(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => Enumeration.get_values('IPRI').first, :subject => 'Assignment test', :description => 'Assignment test', :category_id => 1) assert_equal IssueCategory.find(1).assigned_to, issue.assigned_to diff --git a/vendor/plugins/acts_as_customizable/init.rb b/vendor/plugins/acts_as_customizable/init.rb new file mode 100644 index 00000000..9036aa57 --- /dev/null +++ b/vendor/plugins/acts_as_customizable/init.rb @@ -0,0 +1,2 @@ +require File.dirname(__FILE__) + '/lib/acts_as_customizable' +ActiveRecord::Base.send(:include, Redmine::Acts::Customizable) diff --git a/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb b/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb new file mode 100644 index 00000000..05857d0a --- /dev/null +++ b/vendor/plugins/acts_as_customizable/lib/acts_as_customizable.rb @@ -0,0 +1,82 @@ +# redMine - project management software +# Copyright (C) 2006-2008 Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +module Redmine + module Acts + module Customizable + def self.included(base) + base.extend ClassMethods + end + + module ClassMethods + def acts_as_customizable(options = {}) + return if self.included_modules.include?(Redmine::Acts::Customizable::InstanceMethods) + cattr_accessor :customizable_options + self.customizable_options = options + has_many :custom_values, :dependent => :delete_all, :as => :customized + before_validation_on_create { |customized| customized.custom_field_values } + # Trigger validation only if custom values were changed + validates_associated :custom_values, :on => :update, :if => Proc.new { |customized| customized.custom_field_values_changed? } + send :include, Redmine::Acts::Customizable::InstanceMethods + # Save custom values when saving the customized object + after_save :save_custom_field_values + end + end + + module InstanceMethods + def self.included(base) + base.extend ClassMethods + end + + def available_custom_fields + CustomField.find(:all, :conditions => "type = '#{self.class.name}CustomField'", + :order => 'position') + end + + def custom_field_values=(values) + @custom_field_values_changed = true + values = values.stringify_keys + custom_field_values.each do |custom_value| + custom_value.value = values[custom_value.custom_field_id.to_s] if values.has_key?(custom_value.custom_field_id.to_s) + end if values.is_a?(Hash) + end + + def custom_field_values + @custom_field_values ||= available_custom_fields.collect { |x| custom_values.detect { |v| v.custom_field == x } || custom_values.build(:custom_field => x, :value => nil) } + end + + def custom_field_values_changed? + @custom_field_values_changed == true + end + + def custom_value_for(c) + field_id = (c.is_a?(CustomField) ? c.id : c.to_i) + custom_values.detect {|v| v.custom_field_id == field_id } + end + + def save_custom_field_values + custom_field_values.each(&:save) + @custom_field_values_changed = false + @custom_field_values = nil + end + + module ClassMethods + end + end + end + end +end