Fixed that custom fields with hidden/read-only combination may be displayed on issue form (#19297).

git-svn-id: http://svn.redmine.org/redmine/trunk@14137 e93f8b46-1217-0410-a6f0-8f06a7374b81

2 files changed, 35 insertions, 4 deletions

diff --git a/app/models/issue.rb b/app/models/issue.rb
index a45ff53f2..7d2bf5bba 100644
--- 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 25ccb0812..5d11f8cc3 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -911,7 +911,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)
@@ -941,6 +941,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]