From d7b669e50b1c863b748231dc8fb66a692a33cdd1 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sun, 15 Jul 2012 14:12:17 +0000 Subject: Workflow enhancement: editable and required fields configurable by role, tracker and status (#703, #3521). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@9977 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/issue.rb | 126 +++++++++++++++++++++++++++++++++++--- app/models/issue_status.rb | 8 +-- app/models/role.rb | 4 +- app/models/tracker.rb | 15 +++-- app/models/workflow.rb | 89 --------------------------- app/models/workflow_permission.rb | 29 +++++++++ app/models/workflow_rule.rb | 73 ++++++++++++++++++++++ app/models/workflow_transition.rb | 39 ++++++++++++ 8 files changed, 272 insertions(+), 111 deletions(-) delete mode 100644 app/models/workflow.rb create mode 100644 app/models/workflow_permission.rb create mode 100644 app/models/workflow_rule.rb create mode 100644 app/models/workflow_transition.rb (limited to 'app/models') diff --git a/app/models/issue.rb b/app/models/issue.rb index 5fc0f1d61..152953708 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -58,7 +58,7 @@ 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 - validate :validate_issue + validate :validate_issue, :validate_required_fields scope :visible, lambda {|*args| { :include => :project, @@ -146,6 +146,11 @@ class Issue < ActiveRecord::Base super end + def reload(*args) + @workflow_rule_by_attribute = nil + super + end + # Overrides Redmine::Acts::Customizable::InstanceMethods#available_custom_fields def available_custom_fields (project && tracker) ? (project.all_issue_custom_fields & tracker.custom_fields.all) : [] @@ -208,7 +213,9 @@ class Issue < ActiveRecord::Base def status_id=(sid) self.status = nil - write_attribute(:status_id, sid) + result = write_attribute(:status_id, sid) + @workflow_rule_by_attribute = nil + result end def priority_id=(pid) @@ -230,6 +237,7 @@ class Issue < ActiveRecord::Base self.tracker = nil result = write_attribute(:tracker_id, tid) @custom_field_values = nil + @workflow_rule_by_attribute = nil result end @@ -336,9 +344,10 @@ class Issue < ActiveRecord::Base :if => lambda {|issue, user| (issue.new_record? || user.allowed_to?(:edit_issues, issue.project)) && user.allowed_to?(:manage_subtasks, issue.project)} - def safe_attribute_names(*args) - names = super(*args) + def safe_attribute_names(user=nil) + names = super names -= disabled_core_fields + names -= read_only_attribute_names(user) names end @@ -362,15 +371,15 @@ class Issue < ActiveRecord::Base self.tracker_id = t end - attrs = delete_unsafe_attributes(attrs, user) - return if attrs.empty? - - if attrs['status_id'] - unless new_statuses_allowed_to(user).collect(&:id).include?(attrs['status_id'].to_i) - attrs.delete('status_id') + if (s = attrs.delete('status_id')) && safe_attribute?('status_id') + if new_statuses_allowed_to(user).collect(&:id).include?(s.to_i) + self.status_id = s end end + attrs = delete_unsafe_attributes(attrs, user) + return if attrs.empty? + unless leaf? attrs.reject! {|k,v| %w(priority_id done_ratio start_date due_date estimated_hours).include?(k)} end @@ -379,6 +388,14 @@ class Issue < ActiveRecord::Base attrs.delete('parent_issue_id') unless Issue.visible(user).exists?(attrs['parent_issue_id'].to_i) end + if attrs['custom_field_values'].present? + attrs['custom_field_values'] = attrs['custom_field_values'].reject {|k, v| read_only_attribute_names(user).include? k.to_s} + end + + if attrs['custom_fields'].present? + attrs['custom_fields'] = attrs['custom_fields'].reject {|c| read_only_attribute_names(user).include? c['id'].to_s} + end + # mass-assignment security bypass assign_attributes attrs, :without_protection => true end @@ -387,6 +404,76 @@ class Issue < ActiveRecord::Base tracker ? tracker.disabled_core_fields : [] end + # Returns the custom_field_values that can be edited by the given user + def editable_custom_field_values(user=nil) + custom_field_values.reject do |value| + read_only_attribute_names(user).include?(value.custom_field_id.to_s) + end + end + + # Returns the names of attributes that are read-only for user or the current user + # For users with multiple roles, the read-only fields are the intersection of + # read-only fields of each role + # The result is an array of strings where sustom fields are represented with their ids + # + # Examples: + # issue.read_only_attribute_names # => ['due_date', '2'] + # issue.read_only_attribute_names(user) # => [] + def read_only_attribute_names(user=nil) + workflow_rule_by_attribute(user).select {|attr, rule| rule == 'readonly'}.keys + end + + # Returns the names of required attributes for user or the current user + # For users with multiple roles, the required fields are the intersection of + # required fields of each role + # The result is an array of strings where sustom fields are represented with their ids + # + # Examples: + # issue.required_attribute_names # => ['due_date', '2'] + # issue.required_attribute_names(user) # => [] + def required_attribute_names(user=nil) + workflow_rule_by_attribute(user).select {|attr, rule| rule == 'required'}.keys + end + + # Returns true if the attribute is required for user + def required_attribute?(name, user=nil) + required_attribute_names(user).include?(name.to_s) + end + + # Returns a hash of the workflow rule by attribute for the given user + # + # Examples: + # issue.workflow_rule_by_attribute # => {'due_date' => 'required', 'start_date' => 'readonly'} + def workflow_rule_by_attribute(user=nil) + return @workflow_rule_by_attribute if @workflow_rule_by_attribute && user.nil? + + user_real = user || User.current + roles = user_real.admin ? Role.all : user_real.roles_for_project(project) + return {} if roles.empty? + + result = {} + workflow_permissions = WorkflowPermission.where(:tracker_id => tracker_id, :old_status_id => status_id, :role_id => roles.map(&:id)).all + if workflow_permissions.any? + workflow_rules = workflow_permissions.inject({}) do |h, wp| + h[wp.field_name] ||= [] + h[wp.field_name] << wp.rule + h + end + workflow_rules.each do |attr, rules| + next if rules.size < roles.size + uniq_rules = rules.uniq + if uniq_rules.size == 1 + result[attr] = uniq_rules.first + else + result[attr] = 'required' + end + end + end + @workflow_rule_by_attribute = result if user.nil? + result + end + private :workflow_rule_by_attribute + def done_ratio if Issue.use_status_for_done_ratio? && status && status.default_done_ratio status.default_done_ratio @@ -448,6 +535,25 @@ class Issue < ActiveRecord::Base end end + # Validates the issue against additional workflow requirements + def validate_required_fields + user = new_record? ? author : current_journal.try(:user) + + required_attribute_names(user).each do |attribute| + if attribute =~ /^\d+$/ + attribute = attribute.to_i + v = custom_field_values.detect {|v| v.custom_field_id == attribute } + if v && v.value.blank? + errors.add :base, v.custom_field.name + ' ' + l('activerecord.errors.messages.blank') + end + else + if respond_to?(attribute) && send(attribute).blank? + errors.add attribute, :blank + end + end + end + end + # Set the done_ratio using the status if that setting is set. This will keep the done_ratios # even if the user turns off the setting later def update_done_ratio_from_issue_status diff --git a/app/models/issue_status.rb b/app/models/issue_status.rb index 6973ca656..2d09d5e88 100644 --- a/app/models/issue_status.rb +++ b/app/models/issue_status.rb @@ -17,10 +17,10 @@ class IssueStatus < ActiveRecord::Base before_destroy :check_integrity - has_many :workflows, :foreign_key => "old_status_id" + has_many :workflows, :class_name => 'WorkflowTransition', :foreign_key => "old_status_id" acts_as_list - before_destroy :delete_workflows + before_destroy :delete_workflow_rules after_save :update_default validates_presence_of :name @@ -98,7 +98,7 @@ private end # Deletes associated workflows - def delete_workflows - Workflow.delete_all(["old_status_id = :id OR new_status_id = :id", {:id => id}]) + def delete_workflow_rules + WorkflowRule.delete_all(["old_status_id = :id OR new_status_id = :id", {:id => id}]) end end diff --git a/app/models/role.rb b/app/models/role.rb index 8e8737afb..412e5a63c 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -47,9 +47,9 @@ class Role < ActiveRecord::Base } before_destroy :check_deletable - has_many :workflows, :dependent => :delete_all do + has_many :workflow_rules, :dependent => :delete_all do def copy(source_role) - Workflow.copy(nil, source_role, nil, proxy_association.owner) + WorkflowRule.copy(nil, source_role, nil, proxy_association.owner) end end diff --git a/app/models/tracker.rb b/app/models/tracker.rb index 109e0f423..472b94b31 100644 --- a/app/models/tracker.rb +++ b/app/models/tracker.rb @@ -17,14 +17,17 @@ class Tracker < ActiveRecord::Base - # Other fields should be appended, not inserted! - CORE_FIELDS = %w(assigned_to_id category_id fixed_version_id parent_issue_id start_date due_date estimated_hours done_ratio) + CORE_FIELDS_UNDISABLABLE = %w(project_id tracker_id subject description priority_id is_private).freeze + # Fields that can be disabled + # Other (future) fields should be appended, not inserted! + CORE_FIELDS = %w(assigned_to_id category_id fixed_version_id parent_issue_id start_date due_date estimated_hours done_ratio).freeze + CORE_FIELDS_ALL = (CORE_FIELDS_UNDISABLABLE + CORE_FIELDS).freeze before_destroy :check_integrity has_many :issues - has_many :workflows, :dependent => :delete_all do + has_many :workflow_rules, :dependent => :delete_all do def copy(source_tracker) - Workflow.copy(source_tracker, nil, proxy_association.owner, nil) + WorkflowRule.copy(source_tracker, nil, proxy_association.owner, nil) end end @@ -56,8 +59,8 @@ class Tracker < ActiveRecord::Base return [] end - ids = Workflow. - connection.select_rows("SELECT DISTINCT old_status_id, new_status_id FROM #{Workflow.table_name} WHERE tracker_id = #{id}"). + ids = WorkflowTransition. + connection.select_rows("SELECT DISTINCT old_status_id, new_status_id FROM #{WorkflowTransition.table_name} WHERE tracker_id = #{id} AND type = 'WorkflowTransition'"). flatten. uniq diff --git a/app/models/workflow.rb b/app/models/workflow.rb deleted file mode 100644 index 36b4c7df8..000000000 --- a/app/models/workflow.rb +++ /dev/null @@ -1,89 +0,0 @@ -# Redmine - project management software -# Copyright (C) 2006-2012 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. - -class Workflow < ActiveRecord::Base - belongs_to :role - belongs_to :old_status, :class_name => 'IssueStatus', :foreign_key => 'old_status_id' - belongs_to :new_status, :class_name => 'IssueStatus', :foreign_key => 'new_status_id' - - validates_presence_of :role, :old_status, :new_status - - # Returns workflow transitions count by tracker and role - def self.count_by_tracker_and_role - counts = connection.select_all("SELECT role_id, tracker_id, count(id) AS c FROM #{Workflow.table_name} GROUP BY role_id, tracker_id") - roles = Role.sorted.all - trackers = Tracker.sorted.all - - result = [] - trackers.each do |tracker| - t = [] - roles.each do |role| - row = counts.detect {|c| c['role_id'].to_s == role.id.to_s && c['tracker_id'].to_s == tracker.id.to_s} - t << [role, (row.nil? ? 0 : row['c'].to_i)] - end - result << [tracker, t] - end - - result - end - - # Copies workflows from source to targets - def self.copy(source_tracker, source_role, target_trackers, target_roles) - unless source_tracker.is_a?(Tracker) || source_role.is_a?(Role) - raise ArgumentError.new("source_tracker or source_role must be specified") - end - - target_trackers = [target_trackers].flatten.compact - target_roles = [target_roles].flatten.compact - - target_trackers = Tracker.sorted.all if target_trackers.empty? - target_roles = Role.all if target_roles.empty? - - target_trackers.each do |target_tracker| - target_roles.each do |target_role| - copy_one(source_tracker || target_tracker, - source_role || target_role, - target_tracker, - target_role) - end - end - end - - # Copies a single set of workflows from source to target - def self.copy_one(source_tracker, source_role, target_tracker, target_role) - unless source_tracker.is_a?(Tracker) && !source_tracker.new_record? && - source_role.is_a?(Role) && !source_role.new_record? && - target_tracker.is_a?(Tracker) && !target_tracker.new_record? && - target_role.is_a?(Role) && !target_role.new_record? - - raise ArgumentError.new("arguments can not be nil or unsaved objects") - end - - if source_tracker == target_tracker && source_role == target_role - false - else - transaction do - delete_all :tracker_id => target_tracker.id, :role_id => target_role.id - connection.insert "INSERT INTO #{Workflow.table_name} (tracker_id, role_id, old_status_id, new_status_id, author, assignee)" + - " SELECT #{target_tracker.id}, #{target_role.id}, old_status_id, new_status_id, author, assignee" + - " FROM #{Workflow.table_name}" + - " WHERE tracker_id = #{source_tracker.id} AND role_id = #{source_role.id}" - end - true - end - end -end diff --git a/app/models/workflow_permission.rb b/app/models/workflow_permission.rb new file mode 100644 index 000000000..72e95543f --- /dev/null +++ b/app/models/workflow_permission.rb @@ -0,0 +1,29 @@ +# Redmine - project management software +# Copyright (C) 2006-2012 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. + +class WorkflowPermission < WorkflowRule + validates_inclusion_of :rule, :in => %w(readonly required) + validate :validate_field_name + + protected + + def validate_field_name + unless Tracker::CORE_FIELDS_ALL.include?(field_name) || field_name.to_s.match(/^\d+$/) + errors.add :field_name, :invalid + end + end +end diff --git a/app/models/workflow_rule.rb b/app/models/workflow_rule.rb new file mode 100644 index 000000000..2fc020ba1 --- /dev/null +++ b/app/models/workflow_rule.rb @@ -0,0 +1,73 @@ +# Redmine - project management software +# Copyright (C) 2006-2012 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. + +class WorkflowRule < ActiveRecord::Base + self.table_name = "#{table_name_prefix}workflows#{table_name_suffix}" + + belongs_to :role + belongs_to :tracker + belongs_to :old_status, :class_name => 'IssueStatus', :foreign_key => 'old_status_id' + belongs_to :new_status, :class_name => 'IssueStatus', :foreign_key => 'new_status_id' + + validates_presence_of :role, :tracker, :old_status + + # Copies workflows from source to targets + def self.copy(source_tracker, source_role, target_trackers, target_roles) + unless source_tracker.is_a?(Tracker) || source_role.is_a?(Role) + raise ArgumentError.new("source_tracker or source_role must be specified") + end + + target_trackers = [target_trackers].flatten.compact + target_roles = [target_roles].flatten.compact + + target_trackers = Tracker.sorted.all if target_trackers.empty? + target_roles = Role.all if target_roles.empty? + + target_trackers.each do |target_tracker| + target_roles.each do |target_role| + copy_one(source_tracker || target_tracker, + source_role || target_role, + target_tracker, + target_role) + end + end + end + + # Copies a single set of workflows from source to target + def self.copy_one(source_tracker, source_role, target_tracker, target_role) + unless source_tracker.is_a?(Tracker) && !source_tracker.new_record? && + source_role.is_a?(Role) && !source_role.new_record? && + target_tracker.is_a?(Tracker) && !target_tracker.new_record? && + target_role.is_a?(Role) && !target_role.new_record? + + raise ArgumentError.new("arguments can not be nil or unsaved objects") + end + + if source_tracker == target_tracker && source_role == target_role + false + else + transaction do + delete_all :tracker_id => target_tracker.id, :role_id => target_role.id + connection.insert "INSERT INTO #{WorkflowRule.table_name} (tracker_id, role_id, old_status_id, new_status_id, author, assignee, field_name, rule, type)" + + " SELECT #{target_tracker.id}, #{target_role.id}, old_status_id, new_status_id, author, assignee, field_name, rule, type" + + " FROM #{WorkflowRule.table_name}" + + " WHERE tracker_id = #{source_tracker.id} AND role_id = #{source_role.id}" + end + true + end + end +end diff --git a/app/models/workflow_transition.rb b/app/models/workflow_transition.rb new file mode 100644 index 000000000..0c01edd04 --- /dev/null +++ b/app/models/workflow_transition.rb @@ -0,0 +1,39 @@ +# Redmine - project management software +# Copyright (C) 2006-2012 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. + +class WorkflowTransition < WorkflowRule + validates_presence_of :new_status + + # Returns workflow transitions count by tracker and role + def self.count_by_tracker_and_role + counts = connection.select_all("SELECT role_id, tracker_id, count(id) AS c FROM #{table_name} WHERE type = 'WorkflowTransition' GROUP BY role_id, tracker_id") + roles = Role.sorted.all + trackers = Tracker.sorted.all + + result = [] + trackers.each do |tracker| + t = [] + roles.each do |role| + row = counts.detect {|c| c['role_id'].to_s == role.id.to_s && c['tracker_id'].to_s == tracker.id.to_s} + t << [role, (row.nil? ? 0 : row['c'].to_i)] + end + result << [tracker, t] + end + + result + end +end -- cgit v1.2.3