Merged r14137 (#19297).

git-svn-id: http://svn.redmine.org/redmine/branches/3.0-stable@14138 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/models/issue.rb b/app/models/issue.rb
index eb1e22d6d3d85492b6dddacbc3421d899a3453fd..233449bdecf711adcb591e7f04f19d1c6d40cb65 100644 (file)
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -548,13 +548,27 @@ class Issue < ActiveRecord::Base
     workflow_permissions = WorkflowPermission.where(:tracker_id => tracker_id, :old_status_id => status_id, :role_id => roles.map(&:id)).to_a
     if workflow_permissions.any?
       workflow_rules = workflow_permissions.inject({}) do |h, wp|
-        h[wp.field_name] ||= []
-        h[wp.field_name] << wp.rule
+        h[wp.field_name] ||= {}
+        h[wp.field_name][wp.role_id] = wp.rule
         h
       end
+      fields_with_roles = {}
+      IssueCustomField.where(:visible => false).joins(:roles).pluck(:id, "role_id").each do |field_id, role_id|
+        fields_with_roles[field_id] ||= []
+        fields_with_roles[field_id] << role_id
+      end
+      roles.each do |role|
+        fields_with_roles.each do |field_id, role_ids|
+          unless role_ids.include?(role.id)
+            field_name = field_id.to_s
+            workflow_rules[field_name] ||= {}
+            workflow_rules[field_name][role.id] = 'readonly'
+          end
+        end
+      end
       workflow_rules.each do |attr, rules|
         next if rules.size < roles.size
-        uniq_rules = rules.uniq
+        uniq_rules = rules.values.uniq
         if uniq_rules.size == 1
           result[attr] = uniq_rules.first
         else

diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index 72157e5381ace1d46b70a08dbc44cac393960a6f..61b24c10d388420d8f94238cf2d4d472d61eb293 100644 (file)
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -888,7 +888,7 @@ class IssueTest < ActiveSupport::TestCase
     assert_equal [], issue.required_attribute_names(user.reload)
 
     WorkflowPermission.create!(:old_status_id => 1, :tracker_id => 1,
-                               :role_id => 2, :field_name => 'due_date',
+                               :role_id => 3, :field_name => 'due_date',
                                :rule => 'readonly')
     # required + readonly => required
     assert_equal %w(due_date), issue.required_attribute_names(user)
@@ -918,6 +918,23 @@ class IssueTest < ActiveSupport::TestCase
     assert_equal %w(due_date), issue.read_only_attribute_names(user)
   end
 
+  # A field that is not visible by role 2 and readonly by role 1 should be readonly for user with role 1 and 2
+  def test_read_only_attribute_names_should_include_custom_fields_that_combine_readonly_and_not_visible_for_roles
+    field = IssueCustomField.generate!(
+      :is_for_all => true, :trackers => Tracker.all, :visible => false, :role_ids => [1]
+    )
+    WorkflowPermission.delete_all
+    WorkflowPermission.create!(
+      :old_status_id => 1, :tracker_id => 1, :role_id => 1, :field_name => field.id, :rule => 'readonly'
+    )
+    user = User.generate!
+    project = Project.find(1)
+    User.add_to_project(user, project, Role.where(:id => [1, 2]))
+
+    issue = Issue.new(:project_id => 1, :tracker_id => 1, :status_id => 1)
+    assert_equal [field.id.to_s], issue.read_only_attribute_names(user)
+  end
+
   def test_workflow_rules_should_ignore_roles_without_issue_permissions
     role = Role.generate! :permissions => [:view_issues, :edit_issues]
     ignored_role = Role.generate! :permissions => [:view_issues]