From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Fri, 20 Mar 2015 09:52:00 +0000 (+0000)
Subject: Backported r14137 (#19297).
X-Git-Tag: 2.6.4~12
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=2b83ffe2844cf4bdc93697f1423a0164e8e83920;p=redmine.git

Backported r14137 (#19297).

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

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 375dfb29d..3d9627c1f 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -533,13 +533,29 @@ class Issue < ActiveRecord::Base
     workflow_permissions = WorkflowPermission.where(:tracker_id => tracker_id, :old_status_id => status_id, :role_id => roles.map(&:id))
     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 = {}
+      # #pluck with 2 arguments is not supported in Rails 3.2
+      sql = IssueCustomField.where(:visible => false).joins(:roles).select("#{CustomField.table_name}.id, role_id").to_sql
+      self.class.connection.select_rows(sql).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 0008a3779..b04907c5e 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -878,7 +878,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)
@@ -908,6 +908,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]).all)
+
+    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_copy
     issue = Issue.new.copy_from(1)
     assert issue.copy?