Role-based issue custom field visibility (#5037).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@12012 e93f8b46-1217-0410-a6f0-8f06a7374b81

26 files changed, 253 insertions, 72 deletions

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index 0546db36b..b06e635e0 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -103,6 +103,7 @@ class IssuesController < ApplicationController
     @journals = @issue.journals.includes(:user, :details).reorder("#{Journal.table_name}.id ASC").all
     @journals.each_with_index {|j,i| j.indice = i+1}
     @journals.reject!(&:private_notes?) unless User.current.allowed_to?(:view_private_notes, @issue.project)
+    @journals.select! {|journal| journal.notes? || journal.visible_details.any?}
     @journals.reverse! if User.current.wants_comments_in_reverse_order?
 
     @changesets = @issue.changesets.visible.all
@@ -230,7 +231,7 @@ class IssuesController < ApplicationController
     else
       @available_statuses = @issues.map(&:new_statuses_allowed_to).reduce(:&)
     end
-    @custom_fields = target_projects.map{|p|p.all_issue_custom_fields}.reduce(:&)
+    @custom_fields = target_projects.map{|p|p.all_issue_custom_fields.visible}.reduce(:&)
     @assignables = target_projects.map(&:assignable_users).reduce(:&)
     @trackers = target_projects.map(&:trackers).reduce(:&)
     @versions = target_projects.map {|p| p.shared_versions.open}.reduce(:&)
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index f88a61e21..2e45d2e97 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -160,12 +160,13 @@ module IssuesHelper
   end
 
   def render_custom_fields_rows(issue)
-    return if issue.custom_field_values.empty?
+    values = issue.visible_custom_field_values
+    return if values.empty?
     ordered_values = []
-    half = (issue.custom_field_values.size / 2.0).ceil
+    half = (values.size / 2.0).ceil
     half.times do |i|
-      ordered_values << issue.custom_field_values[i]
-      ordered_values << issue.custom_field_values[i + half]
+      ordered_values << values[i]
+      ordered_values << values[i + half]
     end
     s = "<tr>\n"
     n = 0
diff --git a/app/helpers/workflows_helper.rb b/app/helpers/workflows_helper.rb
index 1cec67a88..7ef6e9942 100644
--- a/app/helpers/workflows_helper.rb
+++ b/app/helpers/workflows_helper.rb
@@ -22,11 +22,20 @@ module WorkflowsHelper
     field.is_a?(CustomField) ? field.is_required? : %w(project_id tracker_id subject priority_id is_private).include?(field)
   end
 
-  def field_permission_tag(permissions, status, field)
+  def field_permission_tag(permissions, status, field, role)
     name = field.is_a?(CustomField) ? field.id.to_s : field
     options = [["", ""], [l(:label_readonly), "readonly"]]
     options << [l(:label_required), "required"] unless field_required?(field)
+    html_options = {}
+    selected = permissions[status.id][name]
 
-    select_tag("permissions[#{name}][#{status.id}]", options_for_select(options, permissions[status.id][name]))
+    hidden = field.is_a?(CustomField) && !field.visible? && !role.custom_fields.to_a.include?(field)
+    if hidden
+      options[0][0] = l(:label_hidden)
+      selected = ''
+      html_options[:disabled] = true
+    end
+
+    select_tag("permissions[#{name}][#{status.id}]", options_for_select(options, selected), html_options)
   end
 end
diff --git a/app/models/custom_field.rb b/app/models/custom_field.rb
index c15192388..c2b717d0e 100644
--- a/app/models/custom_field.rb
+++ b/app/models/custom_field.rb
@@ -19,6 +19,7 @@ class CustomField < ActiveRecord::Base
   include Redmine::SubclassFactory
 
   has_many :custom_values, :dependent => :delete_all
+  has_and_belongs_to_many :roles, :join_table => "#{table_name_prefix}custom_fields_roles#{table_name_suffix}", :foreign_key => "custom_field_id"
   acts_as_list :scope => 'type = \'#{self.class}\''
   serialize :possible_values
 
@@ -26,12 +27,31 @@ class CustomField < ActiveRecord::Base
   validates_uniqueness_of :name, :scope => :type
   validates_length_of :name, :maximum => 30
   validates_inclusion_of :field_format, :in => Redmine::CustomFieldFormat.available_formats
-
   validate :validate_custom_field
+
   before_validation :set_searchable
   after_save :handle_multiplicity_change
+  after_save do |field|
+    if field.visible_changed? && field.visible
+      field.roles.clear
+    end
+  end
 
   scope :sorted, lambda { order("#{table_name}.position ASC") }
+  scope :visible, lambda {|*args|
+    user = args.shift || User.current
+    if user.admin?
+      # nop
+    elsif user.memberships.any?
+      where("#{table_name}.visible = ? OR #{table_name}.id IN (SELECT DISTINCT cfr.custom_field_id FROM #{Member.table_name} m" +
+        " INNER JOIN #{MemberRole.table_name} mr ON mr.member_id = m.id" +
+        " INNER JOIN #{table_name_prefix}custom_fields_roles#{table_name_suffix} cfr ON cfr.role_id = mr.role_id" +
+        " WHERE m.user_id = ?)",
+        true, user.id)
+    else
+      where(:visible => true)
+    end
+  }
 
   CUSTOM_FIELDS_TABS = [
     {:name => 'IssueCustomField', :partial => 'custom_fields/index',
@@ -215,6 +235,7 @@ class CustomField < ActiveRecord::Base
           " ON #{join_alias}.customized_type = '#{self.class.customized_class.base_class.name}'" +
           " AND #{join_alias}.customized_id = #{self.class.customized_class.table_name}.id" +
           " AND #{join_alias}.custom_field_id = #{id}" +
+          " AND (#{visibility_by_project_condition})" +
           " AND #{join_alias}.value <> ''" +
           " AND #{join_alias}.id = (SELECT max(#{join_alias}_2.id) FROM #{CustomValue.table_name} #{join_alias}_2" +
             " WHERE #{join_alias}_2.customized_type = #{join_alias}.customized_type" +
@@ -227,6 +248,7 @@ class CustomField < ActiveRecord::Base
           " ON #{join_alias}.customized_type = '#{self.class.customized_class.base_class.name}'" +
           " AND #{join_alias}.customized_id = #{self.class.customized_class.table_name}.id" +
           " AND #{join_alias}.custom_field_id = #{id}" +
+          " AND (#{visibility_by_project_condition})" +
           " AND #{join_alias}.value <> ''" +
           " AND #{join_alias}.id = (SELECT max(#{join_alias}_2.id) FROM #{CustomValue.table_name} #{join_alias}_2" +
             " WHERE #{join_alias}_2.customized_type = #{join_alias}.customized_type" +
@@ -237,6 +259,7 @@ class CustomField < ActiveRecord::Base
           " ON #{join_alias}.customized_type = '#{self.class.customized_class.base_class.name}'" +
           " AND #{join_alias}.customized_id = #{self.class.customized_class.table_name}.id" +
           " AND #{join_alias}.custom_field_id = #{id}" +
+          " AND (#{visibility_by_project_condition})" +
           " AND #{join_alias}.id = (SELECT max(#{join_alias}_2.id) FROM #{CustomValue.table_name} #{join_alias}_2" +
             " WHERE #{join_alias}_2.customized_type = #{join_alias}.customized_type" +
             " AND #{join_alias}_2.customized_id = #{join_alias}.customized_id" +
@@ -254,6 +277,33 @@ class CustomField < ActiveRecord::Base
     join_alias + "_" + field_format
   end
 
+  def visibility_by_project_condition(project_key=nil, user=User.current)
+    if visible? || user.admin?
+      "1=1"
+    elsif user.anonymous?
+      "1=0"
+    else
+      project_key ||= "#{self.class.customized_class.table_name}.project_id"
+      "#{project_key} IN (SELECT DISTINCT m.project_id FROM #{Member.table_name} m" + 
+        " INNER JOIN #{MemberRole.table_name} mr ON mr.member_id = m.id" +
+        " INNER JOIN #{table_name_prefix}custom_fields_roles#{table_name_suffix} cfr ON cfr.role_id = mr.role_id" +
+        " WHERE m.user_id = #{user.id} AND cfr.custom_field_id = #{id})"
+    end
+  end
+
+  def self.visibility_condition
+    if user.admin?
+      "1=1"
+    elsif user.anonymous?
+      "#{table_name}.visible"
+    else
+      "#{project_key} IN (SELECT DISTINCT m.project_id FROM #{Member.table_name} m" + 
+        " INNER JOIN #{MemberRole.table_name} mr ON mr.member_id = m.id" +
+        " INNER JOIN #{table_name_prefix}custom_fields_roles#{table_name_suffix} cfr ON cfr.role_id = mr.role_id" +
+        " WHERE m.user_id = #{user.id} AND cfr.custom_field_id = #{id})"
+    end
+  end
+
   def <=>(field)
     position <=> field.position
   end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index ccedcfd4c..69ace9f31 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -198,6 +198,13 @@ class Issue < ActiveRecord::Base
     (project && tracker) ? (project.all_issue_custom_fields & tracker.custom_fields.all) : []
   end
 
+  def visible_custom_field_values(user=nil)
+    user_real = user || User.current
+    custom_field_values.select do |value|
+      value.custom_field.visible_by?(project, user_real)
+    end
+  end
+
   # Copies attributes from another issue, arg can be an id or an Issue
   def copy_from(arg, options={})
     issue = arg.is_a?(Issue) ? arg : Issue.visible.find(arg)
@@ -445,11 +452,13 @@ class Issue < ActiveRecord::Base
     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}
+      editable_custom_field_ids = editable_custom_field_values(user).map {|v| v.custom_field_id.to_s}
+      attrs['custom_field_values'] = attrs['custom_field_values'].select {|k, v| editable_custom_field_ids.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}
+      editable_custom_field_ids = editable_custom_field_values(user).map {|v| v.custom_field_id.to_s}
+      attrs['custom_fields'] = attrs['custom_fields'].select {|c| editable_custom_field_ids.include? c['id'].to_s}
     end
 
     # mass-assignment security bypass
@@ -462,7 +471,7 @@ class Issue < ActiveRecord::Base
 
   # 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|
+    visible_custom_field_values(user).reject do |value|
       read_only_attribute_names(user).include?(value.custom_field_id.to_s)
     end
   end
@@ -790,6 +799,21 @@ class Issue < ActiveRecord::Base
     notified_users.collect(&:mail)
   end
 
+  def each_notification(users, &block)
+    if users.any?
+      if custom_field_values.detect {|value| !value.custom_field.visible?}
+        users_by_custom_field_visibility = users.group_by do |user|
+          visible_custom_field_values(user).map(&:custom_field_id).sort
+        end
+        users_by_custom_field_visibility.values.each do |users|
+          yield(users)
+        end
+      else
+        yield(users)
+      end
+    end
+  end
+
   # Returns the number of hours spent on this issue
   def spent_hours
     @spent_hours ||= time_entries.sum(:hours) || 0
diff --git a/app/models/issue_custom_field.rb b/app/models/issue_custom_field.rb
index e7ea9c0b3..9ddedc882 100644
--- a/app/models/issue_custom_field.rb
+++ b/app/models/issue_custom_field.rb
@@ -23,5 +23,13 @@ class IssueCustomField < CustomField
   def type_name
     :label_issue_plural
   end
-end
 
+  def visible_by?(project, user=User.current)
+    visible? || user.admin? || (roles & user.roles_for_project(project)).present?
+  end
+
+  def validate_custom_field
+    super
+    errors.add(:base, l(:label_role_plural) + ' ' + l('activerecord.errors.messages.blank')) unless visible? || roles.present?
+  end
+end
diff --git a/app/models/issue_observer.rb b/app/models/issue_observer.rb
index a75194286..58b636031 100644
--- a/app/models/issue_observer.rb
+++ b/app/models/issue_observer.rb
@@ -17,6 +17,6 @@
 
 class IssueObserver < ActiveRecord::Observer
   def after_create(issue)
-    Mailer.issue_add(issue).deliver if Setting.notified_events.include?('issue_added')
+    Mailer.deliver_issue_add(issue) if Setting.notified_events.include?('issue_added')
   end
 end
diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb
index b2e470d7a..323e46f4c 100644
--- a/app/models/issue_query.rb
+++ b/app/models/issue_query.rb
@@ -225,8 +225,8 @@ class IssueQuery < Query
     @available_columns = self.class.available_columns.dup
     @available_columns += (project ?
                             project.all_issue_custom_fields :
-                            IssueCustomField.all
-                           ).collect {|cf| QueryCustomFieldColumn.new(cf) }
+                            IssueCustomField
+                           ).visible.collect {|cf| QueryCustomFieldColumn.new(cf) }
 
     if User.current.allowed_to?(:view_time_entries, project, :global => true)
       index = nil
diff --git a/app/models/journal.rb b/app/models/journal.rb
index a75c112db..c14051f83 100644
--- a/app/models/journal.rb
+++ b/app/models/journal.rb
@@ -53,6 +53,18 @@ class Journal < ActiveRecord::Base
     (details.empty? && notes.blank?) ? false : super
   end
 
+  def visible_details(user=User.current)
+    details.select do |detail|
+      if detail.property == 'cf' 
+        field_id = detail.prop_key
+        field = CustomField.find_by_id(field_id)
+        field && field.visible_by?(project, user)
+      else
+        true
+      end
+    end
+  end
+
   # Returns the new status if the journal contains a status change, otherwise nil
   def new_status
     c = details.detect {|detail| detail.prop_key == 'status_id'}
@@ -93,20 +105,28 @@ class Journal < ActiveRecord::Base
     @notify = arg
   end
 
-  def recipients
+  def notified_users
     notified = journalized.notified_users
     if private_notes?
       notified = notified.select {|user| user.allowed_to?(:view_private_notes, journalized.project)}
     end
-    notified.map(&:mail)
+    notified
   end
 
-  def watcher_recipients
+  def recipients
+    notified_users.map(&:mail)
+  end
+
+  def notified_watchers
     notified = journalized.notified_watchers
     if private_notes?
       notified = notified.select {|user| user.allowed_to?(:view_private_notes, journalized.project)}
     end
-    notified.map(&:mail)
+    notified
+  end
+
+  def watcher_recipients
+    notified_watchers.map(&:mail)
   end
 
   private
diff --git a/app/models/journal_observer.rb b/app/models/journal_observer.rb
index fe937de07..ff4b89837 100644
--- a/app/models/journal_observer.rb
+++ b/app/models/journal_observer.rb
@@ -23,7 +23,7 @@ class JournalObserver < ActiveRecord::Observer
           (Setting.notified_events.include?('issue_status_updated') && journal.new_status.present?) ||
           (Setting.notified_events.include?('issue_priority_updated') && journal.new_value_for('priority_id').present?)
         )
-      Mailer.issue_edit(journal).deliver
+      Mailer.deliver_issue_edit(journal)
     end
   end
 end
diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb
index 90850dffb..fb30374bd 100644
--- a/app/models/mail_handler.rb
+++ b/app/models/mail_handler.rb
@@ -136,7 +136,7 @@ class MailHandler < ActionMailer::Base
 
   private
 
-  MESSAGE_ID_RE = %r{^<?redmine\.([a-z0-9_]+)\-(\d+)\.\d+@}
+  MESSAGE_ID_RE = %r{^<?redmine\.([a-z0-9_]+)\-(\d+)\.\d+(\.[a-f0-9]+)?@}
   ISSUE_REPLY_SUBJECT_RE = %r{\[[^\]]*#(\d+)\]}
   MESSAGE_REPLY_SUBJECT_RE = %r{\[[^\]]*msg(\d+)\]}
 
diff --git a/app/models/mailer.rb b/app/models/mailer.rb
index 6e07d61f7..6a8aded13 100644
--- a/app/models/mailer.rb
+++ b/app/models/mailer.rb
@@ -27,34 +27,35 @@ class Mailer < ActionMailer::Base
     { :host => Setting.host_name, :protocol => Setting.protocol }
   end
 
-  # Builds a Mail::Message object used to email recipients of the added issue.
-  #
-  # Example:
-  #   issue_add(issue) => Mail::Message object
-  #   Mailer.issue_add(issue).deliver => sends an email to issue recipients
-  def issue_add(issue)
+  # Builds a mail for notifying to_users and cc_users about a new issue
+  def issue_add(issue, to_users, cc_users)
     redmine_headers 'Project' => issue.project.identifier,
                     'Issue-Id' => issue.id,
                     'Issue-Author' => issue.author.login
     redmine_headers 'Issue-Assignee' => issue.assigned_to.login if issue.assigned_to
     message_id issue
+    references issue
     @author = issue.author
     @issue = issue
+    @users = to_users + cc_users
     @issue_url = url_for(:controller => 'issues', :action => 'show', :id => issue)
-    recipients = issue.recipients
-    cc = issue.watcher_recipients - recipients
-    mail :to => recipients,
-      :cc => cc,
+    mail :to => to_users.map(&:mail),
+      :cc => cc_users.map(&:mail),
       :subject => "[#{issue.project.name} - #{issue.tracker.name} ##{issue.id}] (#{issue.status.name}) #{issue.subject}"
   end
 
-  # Builds a Mail::Message object used to email recipients of the edited issue.
-  #
-  # Example:
-  #   issue_edit(journal) => Mail::Message object
-  #   Mailer.issue_edit(journal).deliver => sends an email to issue recipients
-  def issue_edit(journal)
-    issue = journal.journalized.reload
+  # Notifies users about a new issue
+  def self.deliver_issue_add(issue)
+    to = issue.notified_users
+    cc = issue.notified_watchers - to
+    issue.each_notification(to + cc) do |users|
+      Mailer.issue_add(issue, to & users, cc & users).deliver
+    end
+  end
+
+  # Builds a mail for notifying to_users and cc_users about an issue update
+  def issue_edit(journal, to_users, cc_users)
+    issue = journal.journalized
     redmine_headers 'Project' => issue.project.identifier,
                     'Issue-Id' => issue.id,
                     'Issue-Author' => issue.author.login
@@ -62,20 +63,30 @@ class Mailer < ActionMailer::Base
     message_id journal
     references issue
     @author = journal.user
-    recipients = journal.recipients
-    # Watchers in cc
-    cc = journal.watcher_recipients - recipients
     s = "[#{issue.project.name} - #{issue.tracker.name} ##{issue.id}] "
     s << "(#{issue.status.name}) " if journal.new_value_for('status_id')
     s << issue.subject
     @issue = issue
+    @users = to_users + cc_users
     @journal = journal
+    @journal_details = journal.visible_details(@users.first)
     @issue_url = url_for(:controller => 'issues', :action => 'show', :id => issue, :anchor => "change-#{journal.id}")
-    mail :to => recipients,
-      :cc => cc,
+    mail :to => to_users.map(&:mail),
+      :cc => cc_users.map(&:mail),
       :subject => s
   end
 
+  # Notifies users about an issue update
+  def self.deliver_issue_edit(journal)
+    issue = journal.journalized.reload
+    to = journal.notified_users
+    cc = journal.notified_watchers
+    issue.each_notification(to + cc) do |users|
+      next unless journal.notes? || journal.visible_details(users.first).any?
+      Mailer.issue_edit(journal, to & users, cc & users).deliver
+    end
+  end
+
   def reminder(user, issues, days)
     set_language_if_valid user.language
     @issues = issues
@@ -142,6 +153,7 @@ class Mailer < ActionMailer::Base
     redmine_headers 'Project' => news.project.identifier
     @author = news.author
     message_id news
+    references news
     @news = news
     @news_url = url_for(:controller => 'news', :action => 'show', :id => news)
     mail :to => news.recipients,
@@ -158,6 +170,7 @@ class Mailer < ActionMailer::Base
     redmine_headers 'Project' => news.project.identifier
     @author = comment.author
     message_id comment
+    references news
     @news = news
     @comment = comment
     @news_url = url_for(:controller => 'news', :action => 'show', :id => news)
@@ -176,7 +189,7 @@ class Mailer < ActionMailer::Base
                     'Topic-Id' => (message.parent_id || message.id)
     @author = message.author
     message_id message
-    references message.parent unless message.parent.nil?
+    references message.root
     recipients = message.recipients
     cc = ((message.root.watcher_recipients + message.board.watcher_recipients).uniq - recipients)
     @message = message
@@ -386,7 +399,7 @@ class Mailer < ActionMailer::Base
       headers[:message_id] = "<#{self.class.message_id_for(@message_id_object)}>"
     end
     if @references_objects
-      headers[:references] = @references_objects.collect {|o| "<#{self.class.message_id_for(o)}>"}.join(' ')
+      headers[:references] = @references_objects.collect {|o| "<#{self.class.references_for(o)}>"}.join(' ')
     end
 
     super headers do |format|
@@ -434,15 +447,30 @@ class Mailer < ActionMailer::Base
     h.each { |k,v| headers["X-Redmine-#{k}"] = v.to_s }
   end
 
-  # Returns a predictable Message-Id for the given object
-  def self.message_id_for(object)
-    # id + timestamp should reduce the odds of a collision
-    # as far as we don't send multiple emails for the same object
+  def self.token_for(object, rand=true)
     timestamp = object.send(object.respond_to?(:created_on) ? :created_on : :updated_on)
-    hash = "redmine.#{object.class.name.demodulize.underscore}-#{object.id}.#{timestamp.strftime("%Y%m%d%H%M%S")}"
+    hash = [
+      "redmine",
+      "#{object.class.name.demodulize.underscore}-#{object.id}",
+      timestamp.strftime("%Y%m%d%H%M%S")
+    ]
+    if rand
+      hash << Redmine::Utils.random_hex(8)
+    end
     host = Setting.mail_from.to_s.gsub(%r{^.*@}, '')
     host = "#{::Socket.gethostname}.redmine" if host.empty?
-    "#{hash}@#{host}"
+    "#{hash.join('.')}@#{host}"
+  end
+
+  # Returns a Message-Id for the given object
+  def self.message_id_for(object)
+    token_for(object, true)
+  end
+
+  # Returns a uniq token for a given object referenced by all notifications
+  # related to this object
+  def self.references_for(object)
+    token_for(object, false)
   end
 
   def message_id(object)
diff --git a/app/models/query.rb b/app/models/query.rb
index 2753c2768..641a263c6 100644
--- a/app/models/query.rb
+++ b/app/models/query.rb
@@ -81,8 +81,12 @@ class QueryCustomFieldColumn < QueryColumn
   end
 
   def value(object)
-    cv = object.custom_values.select {|v| v.custom_field_id == @cf.id}.collect {|v| @cf.cast_value(v.value)}
-    cv.size > 1 ? cv.sort {|a,b| a.to_s <=> b.to_s} : cv.first
+    if custom_field.visible_by?(object.project, User.current)
+      cv = object.custom_values.select {|v| v.custom_field_id == @cf.id}.collect {|v| @cf.cast_value(v.value)}
+      cv.size > 1 ? cv.sort {|a,b| a.to_s <=> b.to_s} : cv.first
+    else
+      nil
+    end
   end
 
   def css_classes
@@ -560,6 +564,11 @@ class Query < ActiveRecord::Base
       end
     end if filters and valid?
 
+    if (c = group_by_column) && c.is_a?(QueryCustomFieldColumn)
+      # Excludes results for which the grouped custom field is not visible
+      filters_clauses << c.custom_field.visibility_by_project_condition
+    end
+
     filters_clauses << project_statement
     filters_clauses.reject!(&:blank?)
 
@@ -596,7 +605,10 @@ class Query < ActiveRecord::Base
     if operator =~ /[<>]/
       where = "(#{where}) AND #{db_table}.#{db_field} <> ''"
     end
-    "#{queried_table_name}.#{customized_key} #{not_in} IN (SELECT #{customized_class.table_name}.id FROM #{customized_class.table_name} LEFT OUTER JOIN #{db_table} ON #{db_table}.customized_type='#{customized_class}' AND #{db_table}.customized_id=#{customized_class.table_name}.id AND #{db_table}.custom_field_id=#{custom_field_id} WHERE #{where})"
+    "#{queried_table_name}.#{customized_key} #{not_in} IN (" +
+      "SELECT #{customized_class.table_name}.id FROM #{customized_class.table_name}" +
+      " LEFT OUTER JOIN #{db_table} ON #{db_table}.customized_type='#{customized_class}' AND #{db_table}.customized_id=#{customized_class.table_name}.id AND #{db_table}.custom_field_id=#{custom_field_id}" +
+      " WHERE (#{where}) AND (#{filter[:field].visibility_by_project_condition}))"
   end
 
   # Helper method to generate the WHERE sql for a +field+, +operator+ and a +value+
@@ -785,14 +797,14 @@ class Query < ActiveRecord::Base
 
   # Adds filters for the given custom fields scope
   def add_custom_fields_filters(scope, assoc=nil)
-    scope.where(:is_filter => true).sorted.each do |field|
+    scope.visible.where(:is_filter => true).sorted.each do |field|
       add_custom_field_filter(field, assoc)
     end
   end
 
   # Adds filters for the given associations custom fields
   def add_associations_custom_fields_filters(*associations)
-    fields_by_class = CustomField.where(:is_filter => true).group_by(&:class)
+    fields_by_class = CustomField.visible.where(:is_filter => true).group_by(&:class)
     associations.each do |assoc|
       association_klass = queried_class.reflect_on_association(assoc).klass
       fields_by_class.each do |field_class, fields|
diff --git a/app/models/role.rb b/app/models/role.rb
index d24103663..751ec6eb4 100644
--- a/app/models/role.rb
+++ b/app/models/role.rb
@@ -52,6 +52,7 @@ class Role < ActiveRecord::Base
       WorkflowRule.copy(nil, source_role, nil, proxy_association.owner)
     end
   end
+  has_and_belongs_to_many :custom_fields, :join_table => "#{table_name_prefix}custom_fields_roles#{table_name_suffix}", :foreign_key => "role_id"
 
   has_many :member_roles, :dependent => :destroy
   has_many :members, :through => :member_roles
diff --git a/app/models/time_entry_query.rb b/app/models/time_entry_query.rb
index dbccea03d..3a6ab64ef 100644
--- a/app/models/time_entry_query.rb
+++ b/app/models/time_entry_query.rb
@@ -91,8 +91,8 @@ class TimeEntryQuery < Query
   def available_columns
     return @available_columns if @available_columns
     @available_columns = self.class.available_columns.dup
-    @available_columns += TimeEntryCustomField.all.map {|cf| QueryCustomFieldColumn.new(cf) }
-    @available_columns += IssueCustomField.all.map {|cf| QueryAssociationCustomFieldColumn.new(:issue, cf) }
+    @available_columns += TimeEntryCustomField.visible.all.map {|cf| QueryCustomFieldColumn.new(cf) }
+    @available_columns += IssueCustomField.visible.all.map {|cf| QueryAssociationCustomFieldColumn.new(:issue, cf) }
     @available_columns
   end
 
diff --git a/app/views/custom_fields/_form.html.erb b/app/views/custom_fields/_form.html.erb
index b6f7ddb1d..4c0b70243 100644
--- a/app/views/custom_fields/_form.html.erb
+++ b/app/views/custom_fields/_form.html.erb
@@ -64,6 +64,24 @@ when "IssueCustomField" %>
     <p><%= f.check_box :is_for_all %></p>
     <p><%= f.check_box :is_filter %></p>
     <p><%= f.check_box :searchable %></p>
+    <p>
+      <label><%= l(:field_visible) %></label>
+      <label class="block">
+        <%= radio_button_tag 'custom_field[visible]', 1, @custom_field.visible?, :id => 'custom_field_visible_on' %>
+        <%= l(:label_visibility_public) %>
+      </label>
+      <label class="block">
+        <%= radio_button_tag 'custom_field[visible]', 0, !@custom_field.visible?, :id => 'custom_field_visible_off' %>
+        <%= l(:label_visibility_roles) %>:
+      </label>
+      <% Role.givable.sorted.each do |role| %>
+        <label class="block custom_field_role" style="padding-left:2em;">
+          <%= check_box_tag 'custom_field[role_ids][]', role.id, @custom_field.roles.include?(role) %>
+          <%= role.name %>
+        </label>
+      <% end %>
+      <%= hidden_field_tag 'custom_field[role_ids][]', '' %>
+    </p>
 
 <% when "UserCustomField" %>
     <p><%= f.check_box :is_required %></p>
@@ -97,3 +115,12 @@ when "IssueCustomField" %>
 </div>
 
 <% include_calendar_headers_tags %>
+
+<%= javascript_tag do %>
+function toggleCustomFieldRoles(){
+  var checked = $("#custom_field_visible_on").is(':checked');
+  $('.custom_field_role input').attr('disabled', checked);
+}
+$("#custom_field_visible_on, #custom_field_visible_off").change(toggleCustomFieldRoles);
+$(document).ready(toggleCustomFieldRoles);
+<% end %>
diff --git a/app/views/issues/_history.html.erb b/app/views/issues/_history.html.erb
index 470a55acf..94d98ccc7 100644
--- a/app/views/issues/_history.html.erb
+++ b/app/views/issues/_history.html.erb
@@ -8,7 +8,7 @@
 
     <% if journal.details.any? %>
     <ul class="details">
-      <% details_to_strings(journal.details).each do |string| %>
+      <% details_to_strings(journal.visible_details).each do |string| %>
        <li><%= string %></li>
       <% end %>
     </ul>
diff --git a/app/views/issues/index.api.rsb b/app/views/issues/index.api.rsb
index 5009ffa7e..c3bcfd74b 100644
--- a/app/views/issues/index.api.rsb
+++ b/app/views/issues/index.api.rsb
@@ -19,7 +19,7 @@ api.array :issues, api_meta(:total_count => @issue_count, :offset => @offset, :l
       api.done_ratio  issue.done_ratio
       api.estimated_hours issue.estimated_hours
 
-      render_api_custom_values issue.custom_field_values, api
+      render_api_custom_values issue.visible_custom_field_values, api
 
       api.created_on issue.created_on
       api.updated_on issue.updated_on
diff --git a/app/views/issues/show.api.rsb b/app/views/issues/show.api.rsb
index ce788a723..3878e71bd 100644
--- a/app/views/issues/show.api.rsb
+++ b/app/views/issues/show.api.rsb
@@ -18,7 +18,7 @@ api.issue do
   api.estimated_hours @issue.estimated_hours
   api.spent_hours(@issue.spent_hours) if User.current.allowed_to?(:view_time_entries, @project)
 
-  render_api_custom_values @issue.custom_field_values, api
+  render_api_custom_values @issue.visible_custom_field_values, api
 
   api.created_on @issue.created_on
   api.updated_on @issue.updated_on
@@ -55,7 +55,7 @@ api.issue do
         api.notes journal.notes
         api.created_on journal.created_on
         api.array :details do
-          journal.details.each do |detail|
+          journal.visible_details.each do |detail|
             api.detail :property => detail.property, :name => detail.prop_key do
               api.old_value detail.old_value
               api.new_value detail.value
diff --git a/app/views/mailer/_issue.html.erb b/app/views/mailer/_issue.html.erb
index 829f8d576..aee365e56 100644
--- a/app/views/mailer/_issue.html.erb
+++ b/app/views/mailer/_issue.html.erb
@@ -7,7 +7,7 @@
 <li><%=l(:field_assigned_to)%>: <%=h issue.assigned_to %></li>
 <li><%=l(:field_category)%>: <%=h issue.category %></li>
 <li><%=l(:field_fixed_version)%>: <%=h issue.fixed_version %></li>
-<% issue.custom_field_values.each do |c| %>
+<% issue.visible_custom_field_values(users.first).each do |c| %>
   <li><%=h c.custom_field.name %>: <%=h show_value(c) %></li>
 <% end %>
 </ul>
diff --git a/app/views/mailer/_issue.text.erb b/app/views/mailer/_issue.text.erb
index 554488049..a2d5a41b2 100644
--- a/app/views/mailer/_issue.text.erb
+++ b/app/views/mailer/_issue.text.erb
@@ -7,7 +7,7 @@
 * <%=l(:field_assigned_to)%>: <%= issue.assigned_to %>
 * <%=l(:field_category)%>: <%= issue.category %>
 * <%=l(:field_fixed_version)%>: <%= issue.fixed_version %>
-<% issue.custom_field_values.each do |c| %>* <%= c.custom_field.name %>: <%= show_value(c) %>
+<% issue.visible_custom_field_values(users.first).each do |c| %>* <%= c.custom_field.name %>: <%= show_value(c) %>
 <% end -%>
 ----------------------------------------
 <%= issue.description %>
diff --git a/app/views/mailer/issue_add.html.erb b/app/views/mailer/issue_add.html.erb
index fb4a2dab6..99fd08d14 100644
--- a/app/views/mailer/issue_add.html.erb
+++ b/app/views/mailer/issue_add.html.erb
@@ -1,3 +1,3 @@
 <%= l(:text_issue_added, :id => "##{@issue.id}", :author => h(@issue.author)) %>
 <hr />
-<%= render :partial => 'issue', :formats => [:html], :locals => { :issue => @issue, :issue_url => @issue_url } %>
+<%= render :partial => 'issue', :formats => [:html], :locals => { :issue => @issue, :users => @users, :issue_url => @issue_url } %>
diff --git a/app/views/mailer/issue_add.text.erb b/app/views/mailer/issue_add.text.erb
index e990ff0d2..6e3b42725 100644
--- a/app/views/mailer/issue_add.text.erb
+++ b/app/views/mailer/issue_add.text.erb
@@ -1,4 +1,4 @@
 <%= l(:text_issue_added, :id => "##{@issue.id}", :author => @issue.author) %>
 
 ----------------------------------------
-<%= render :partial => 'issue', :formats => [:text], :locals => { :issue => @issue, :issue_url => @issue_url } %>
+<%= render :partial => 'issue', :formats => [:text], :locals => { :issue => @issue, :users => @users, :issue_url => @issue_url } %>
diff --git a/app/views/mailer/issue_edit.html.erb b/app/views/mailer/issue_edit.html.erb
index 322251912..e3b6f5c6c 100644
--- a/app/views/mailer/issue_edit.html.erb
+++ b/app/views/mailer/issue_edit.html.erb
@@ -4,11 +4,11 @@
 <%= l(:text_issue_updated, :id => "##{@issue.id}", :author => h(@journal.user)) %>
 
 <ul>
-<% details_to_strings(@journal.details, false, :only_path => false).each do |string| %>
+<% details_to_strings(@journal_details, false, :only_path => false).each do |string| %>
   <li><%= string %></li>
 <% end %>
 </ul>
 
 <%= textilizable(@journal, :notes, :only_path => false) %>
 <hr />
-<%= render :partial => 'issue', :formats => [:html], :locals => { :issue => @issue, :issue_url => @issue_url } %>
+<%= render :partial => 'issue', :formats => [:html], :locals => { :issue => @issue, :users => @users, :issue_url => @issue_url } %>
diff --git a/app/views/mailer/issue_edit.text.erb b/app/views/mailer/issue_edit.text.erb
index 395f8f626..173d2c4fe 100644
--- a/app/views/mailer/issue_edit.text.erb
+++ b/app/views/mailer/issue_edit.text.erb
@@ -1,6 +1,6 @@
 <%= "(#{l(:field_private_notes)}) " if @journal.private_notes? -%><%= l(:text_issue_updated, :id => "##{@issue.id}", :author => @journal.user) %>
 
-<% details_to_strings(@journal.details, true).each do |string| -%>
+<% details_to_strings(@journal_details, true).each do |string| -%>
 <%= string %>
 <% end -%>
 
@@ -9,4 +9,4 @@
 
 <% end -%>
 ----------------------------------------
-<%= render :partial => 'issue', :formats => [:text], :locals => { :issue => @issue, :issue_url => @issue_url } %>
+<%= render :partial => 'issue', :formats => [:text], :locals => { :issue => @issue, :users => @users, :issue_url => @issue_url } %>
diff --git a/app/views/workflows/permissions.html.erb b/app/views/workflows/permissions.html.erb
index b646146b0..6046201d3 100644
--- a/app/views/workflows/permissions.html.erb
+++ b/app/views/workflows/permissions.html.erb
@@ -62,7 +62,7 @@
         </td>
         <% for status in @statuses -%>
         <td align="center" class="<%= @permissions[status.id][field] %>">
-          <%= field_permission_tag(@permissions, status, field) %>
+          <%= field_permission_tag(@permissions, status, field, @role) %>
           <% unless status == @statuses.last %><a href="#" class="repeat-value">&#187;</a><% end %>
         </td>
         <% end -%>
@@ -82,7 +82,7 @@
           </td>
           <% for status in @statuses -%>
           <td align="center" class="<%= @permissions[status.id][field.id.to_s] %>">
-            <%= field_permission_tag(@permissions, status, field) %>
+            <%= field_permission_tag(@permissions, status, field, @role) %>
             <% unless status == @statuses.last %><a href="#" class="repeat-value">&#187;</a><% end %>
           </td>
           <% end -%>