From 628d05629b734371d3e850a95dadf0be30c5ef20 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 13 Jul 2013 09:20:11 +0000 Subject: [PATCH] Role-based issue custom field visibility (#5037). git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@12012 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/issues_controller.rb | 3 +- app/helpers/issues_helper.rb | 9 +- app/helpers/workflows_helper.rb | 13 +- app/models/custom_field.rb | 52 ++- app/models/issue.rb | 30 +- app/models/issue_custom_field.rb | 10 +- app/models/issue_observer.rb | 2 +- app/models/issue_query.rb | 4 +- app/models/journal.rb | 28 +- app/models/journal_observer.rb | 2 +- app/models/mail_handler.rb | 2 +- app/models/mailer.rb | 88 +++-- app/models/query.rb | 22 +- app/models/role.rb | 1 + app/models/time_entry_query.rb | 4 +- app/views/custom_fields/_form.html.erb | 27 ++ app/views/issues/_history.html.erb | 2 +- app/views/issues/index.api.rsb | 2 +- app/views/issues/show.api.rsb | 4 +- app/views/mailer/_issue.html.erb | 2 +- app/views/mailer/_issue.text.erb | 2 +- app/views/mailer/issue_add.html.erb | 2 +- app/views/mailer/issue_add.text.erb | 2 +- app/views/mailer/issue_edit.html.erb | 4 +- app/views/mailer/issue_edit.text.erb | 4 +- app/views/workflows/permissions.html.erb | 4 +- config/locales/en.yml | 1 + config/locales/fr.yml | 1 + ...130713104233_create_custom_fields_roles.rb | 14 + .../lib/acts_as_searchable.rb | 11 +- lib/redmine/export/pdf.rb | 8 +- .../issues_custom_fields_visibility_test.rb | 322 ++++++++++++++++++ .../search_custom_fields_visibility_test.rb | 78 +++++ .../timelog_custom_fields_visibility_test.rb | 113 ++++++ test/functional/workflows_controller_test.rb | 17 + test/test_helper.rb | 16 +- test/unit/custom_field_test.rb | 38 +++ test/unit/issue_custom_field_test.rb | 42 +++ test/unit/lib/redmine/hook_test.rb | 4 +- test/unit/mailer_test.rb | 62 ++-- test/unit/query_test.rb | 22 ++ 41 files changed, 953 insertions(+), 121 deletions(-) create mode 100644 db/migrate/20130713104233_create_custom_fields_roles.rb create mode 100644 test/functional/issues_custom_fields_visibility_test.rb create mode 100644 test/functional/search_custom_fields_visibility_test.rb create mode 100644 test/functional/timelog_custom_fields_visibility_test.rb create mode 100644 test/unit/issue_custom_field_test.rb 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 = "\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{^ 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" %>

<%= f.check_box :is_for_all %>

<%= f.check_box :is_filter %>

<%= f.check_box :searchable %>

+

+ + + + <% Role.givable.sorted.each do |role| %> + + <% end %> + <%= hidden_field_tag 'custom_field[role_ids][]', '' %> +

<% when "UserCustomField" %>

<%= f.check_box :is_required %>

@@ -97,3 +115,12 @@ when "IssueCustomField" %> <% 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? %> 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 @@
  • <%=l(:field_assigned_to)%>: <%=h issue.assigned_to %>
  • <%=l(:field_category)%>: <%=h issue.category %>
  • <%=l(:field_fixed_version)%>: <%=h issue.fixed_version %>
    • -<% issue.custom_field_values.each do |c| %> +<% issue.visible_custom_field_values(users.first).each do |c| %>
  • <%=h c.custom_field.name %>: <%=h show_value(c) %>
    • <% end %> 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)) %>
    -<%= 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)) %> <%= textilizable(@journal, :notes, :only_path => false) %>
    -<%= 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 @@ <% for status in @statuses -%> - <%= field_permission_tag(@permissions, status, field) %> + <%= field_permission_tag(@permissions, status, field, @role) %> <% unless status == @statuses.last %>»<% end %> <% end -%> @@ -82,7 +82,7 @@ <% for status in @statuses -%> - <%= field_permission_tag(@permissions, status, field) %> + <%= field_permission_tag(@permissions, status, field, @role) %> <% unless status == @statuses.last %>»<% end %> <% end -%> diff --git a/config/locales/en.yml b/config/locales/en.yml index 7099de8c4..18221e837 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -885,6 +885,7 @@ en: label_fields_permissions: Fields permissions label_readonly: Read-only label_required: Required + label_hidden: Hidden label_attribute_of_project: "Project's %{name}" label_attribute_of_issue: "Issue's %{name}" label_attribute_of_author: "Author's %{name}" diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 510913a17..13fcbf0f1 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -861,6 +861,7 @@ fr: label_fields_permissions: Permissions sur les champs label_readonly: Lecture label_required: Obligatoire + label_hidden: Caché label_attribute_of_project: "%{name} du projet" label_attribute_of_issue: "%{name} de la demande" label_attribute_of_author: "%{name} de l'auteur" diff --git a/db/migrate/20130713104233_create_custom_fields_roles.rb b/db/migrate/20130713104233_create_custom_fields_roles.rb new file mode 100644 index 000000000..458bfb50b --- /dev/null +++ b/db/migrate/20130713104233_create_custom_fields_roles.rb @@ -0,0 +1,14 @@ +class CreateCustomFieldsRoles < ActiveRecord::Migration + def self.up + create_table :custom_fields_roles, :id => false do |t| + t.column :custom_field_id, :integer, :null => false + t.column :role_id, :integer, :null => false + end + add_index :custom_fields_roles, [:custom_field_id, :role_id], :unique => true, :name => :custom_fields_roles_ids + CustomField.update_all({:visible => true}, {:type => 'IssueCustomField'}) + end + + def self.down + drop_table :custom_fields_roles + end +end diff --git a/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb b/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb index 5a1f36752..3e91c72dc 100644 --- a/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb +++ b/lib/plugins/acts_as_searchable/lib/acts_as_searchable.rb @@ -81,12 +81,13 @@ module Redmine token_clauses = columns.collect {|column| "(LOWER(#{column}) LIKE ?)"} if !options[:titles_only] && searchable_options[:search_custom_fields] - searchable_custom_field_ids = CustomField.where(:type => "#{self.name}CustomField", :searchable => true).pluck(:id) - if searchable_custom_field_ids.any? - custom_field_sql = "#{table_name}.id IN (SELECT customized_id FROM #{CustomValue.table_name}" + + searchable_custom_fields = CustomField.where(:type => "#{self.name}CustomField", :searchable => true) + searchable_custom_fields.each do |field| + sql = "#{table_name}.id IN (SELECT customized_id FROM #{CustomValue.table_name}" + " WHERE customized_type='#{self.name}' AND customized_id=#{table_name}.id AND LOWER(value) LIKE ?" + - " AND #{CustomValue.table_name}.custom_field_id IN (#{searchable_custom_field_ids.join(',')}))" - token_clauses << custom_field_sql + " AND #{CustomValue.table_name}.custom_field_id = #{field.id})" + + " AND #{field.visibility_by_project_condition(searchable_options[:project_key], user)}" + token_clauses << sql end end diff --git a/lib/redmine/export/pdf.rb b/lib/redmine/export/pdf.rb index f720e662a..193319aa5 100644 --- a/lib/redmine/export/pdf.rb +++ b/lib/redmine/export/pdf.rb @@ -256,7 +256,7 @@ module Redmine def fetch_row_values(issue, query, level) query.inline_columns.collect do |column| s = if column.is_a?(QueryCustomFieldColumn) - cv = issue.custom_field_values.detect {|v| v.custom_field_id == column.custom_field.id} + cv = issue.visible_custom_field_values.detect {|v| v.custom_field_id == column.custom_field.id} show_value(cv) else value = issue.send(column.name) @@ -571,8 +571,8 @@ module Redmine right << nil end - half = (issue.custom_field_values.size / 2.0).ceil - issue.custom_field_values.each_with_index do |custom_value, i| + half = (issue.visible_custom_field_values.size / 2.0).ceil + issue.visible_custom_field_values.each_with_index do |custom_value, i| (i < half ? left : right) << [custom_value.custom_field.name, show_value(custom_value)] end @@ -683,7 +683,7 @@ module Redmine pdf.RDMCell(190,5, title) pdf.Ln pdf.SetFontStyle('I',8) - details_to_strings(journal.details, true).each do |string| + details_to_strings(journal.visible_details, true).each do |string| pdf.RDMMultiCell(190,5, "- " + string) end if journal.notes? diff --git a/test/functional/issues_custom_fields_visibility_test.rb b/test/functional/issues_custom_fields_visibility_test.rb new file mode 100644 index 000000000..dfe6e0794 --- /dev/null +++ b/test/functional/issues_custom_fields_visibility_test.rb @@ -0,0 +1,322 @@ +# Redmine - project management software +# Copyright (C) 2006-2013 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. + +require File.expand_path('../../test_helper', __FILE__) + +class IssuesCustomFieldsVisibilityTest < ActionController::TestCase + tests IssuesController + fixtures :projects, + :users, + :roles, + :members, + :member_roles, + :issue_statuses, + :trackers, + :projects_trackers, + :enabled_modules, + :enumerations, + :workflows + + def setup + CustomField.delete_all + Issue.delete_all + field_attributes = {:field_format => 'string', :is_for_all => true, :is_filter => true, :trackers => Tracker.all} + @fields = [] + @fields << (@field1 = IssueCustomField.create!(field_attributes.merge(:name => 'Field 1', :visible => true))) + @fields << (@field2 = IssueCustomField.create!(field_attributes.merge(:name => 'Field 2', :visible => false, :role_ids => [1, 2]))) + @fields << (@field3 = IssueCustomField.create!(field_attributes.merge(:name => 'Field 3', :visible => false, :role_ids => [1, 3]))) + @issue = Issue.generate!( + :author_id => 1, + :project_id => 1, + :tracker_id => 1, + :custom_field_values => {@field1.id => 'Value0', @field2.id => 'Value1', @field3.id => 'Value2'} + ) + + @user_with_role_on_other_project = User.generate! + User.add_to_project(@user_with_role_on_other_project, Project.find(2), Role.find(3)) + + @users_to_test = { + User.find(1) => [@field1, @field2, @field3], + User.find(3) => [@field1, @field2], + @user_with_role_on_other_project => [@field1], # should see field1 only on Project 1 + User.generate! => [@field1], + User.anonymous => [@field1] + } + + Member.where(:project_id => 1).each do |member| + member.destroy unless @users_to_test.keys.include?(member.principal) + end + end + + def test_show_should_show_visible_custom_fields_only + @users_to_test.each do |user, fields| + @request.session[:user_id] = user.id + get :show, :id => @issue.id + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_select 'td', {:text => "Value#{i}", :count => 1}, "User #{user.id} was not able to view #{field.name}" + else + assert_select 'td', {:text => "Value#{i}", :count => 0}, "User #{user.id} was able to view #{field.name}" + end + end + end + end + + def test_show_should_show_visible_custom_fields_only_in_api + @users_to_test.each do |user, fields| + with_settings :rest_api_enabled => '1' do + get :show, :id => @issue.id, :format => 'xml', :include => 'custom_fields', :key => user.api_key + end + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_select "custom_field[id=#{field.id}] value", {:text => "Value#{i}", :count => 1}, "User #{user.id} was not able to view #{field.name} in API" + else + assert_select "custom_field[id=#{field.id}] value", {:text => "Value#{i}", :count => 0}, "User #{user.id} was not able to view #{field.name} in API" + end + end + end + end + + def test_show_should_show_visible_custom_fields_only_in_history + @issue.init_journal(User.find(1)) + @issue.custom_field_values = {@field1.id => 'NewValue0', @field2.id => 'NewValue1', @field3.id => 'NewValue2'} + @issue.save! + + @users_to_test.each do |user, fields| + @request.session[:user_id] = user.id + get :show, :id => @issue.id + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_select 'ul.details i', {:text => "Value#{i}", :count => 1}, "User #{user.id} was not able to view #{field.name} change" + else + assert_select 'ul.details i', {:text => "Value#{i}", :count => 0}, "User #{user.id} was able to view #{field.name} change" + end + end + end + end + + def test_show_should_show_visible_custom_fields_only_in_history_api + @issue.init_journal(User.find(1)) + @issue.custom_field_values = {@field1.id => 'NewValue0', @field2.id => 'NewValue1', @field3.id => 'NewValue2'} + @issue.save! + + @users_to_test.each do |user, fields| + with_settings :rest_api_enabled => '1' do + get :show, :id => @issue.id, :format => 'xml', :include => 'journals', :key => user.api_key + end + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_select 'details old_value', {:text => "Value#{i}", :count => 1}, "User #{user.id} was not able to view #{field.name} change in API" + else + assert_select 'details old_value', {:text => "Value#{i}", :count => 0}, "User #{user.id} was able to view #{field.name} change in API" + end + end + end + end + + def test_edit_should_show_visible_custom_fields_only + Role.anonymous.add_permission! :edit_issues + + @users_to_test.each do |user, fields| + @request.session[:user_id] = user.id + get :edit, :id => @issue.id + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_select 'input[value=?]', "Value#{i}", 1, "User #{user.id} was not able to edit #{field.name}" + else + assert_select 'input[value=?]', "Value#{i}", 0, "User #{user.id} was able to edit #{field.name}" + end + end + end + end + + def test_update_should_update_visible_custom_fields_only + Role.anonymous.add_permission! :edit_issues + + @users_to_test.each do |user, fields| + @request.session[:user_id] = user.id + put :update, :id => @issue.id, + :issue => {:custom_field_values => { + @field1.id.to_s => "User#{user.id}Value0", + @field2.id.to_s => "User#{user.id}Value1", + @field3.id.to_s => "User#{user.id}Value2", + }} + @issue.reload + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_equal "User#{user.id}Value#{i}", @issue.custom_field_value(field), "User #{user.id} was not able to update #{field.name}" + else + assert_not_equal "User#{user.id}Value#{i}", @issue.custom_field_value(field), "User #{user.id} was able to update #{field.name}" + end + end + end + end + + def test_index_should_show_visible_custom_fields_only + @users_to_test.each do |user, fields| + @request.session[:user_id] = user.id + get :index, :c => (["subject"] + @fields.map{|f| "cf_#{f.id}"}) + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_select 'td', {:text => "Value#{i}", :count => 1}, "User #{user.id} was not able to view #{field.name}" + else + assert_select 'td', {:text => "Value#{i}", :count => 0}, "User #{user.id} was able to view #{field.name}" + end + end + end + end + + def test_index_as_csv_should_show_visible_custom_fields_only + @users_to_test.each do |user, fields| + @request.session[:user_id] = user.id + get :index, :c => (["subject"] + @fields.map{|f| "cf_#{f.id}"}), :format => 'csv' + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_include "Value#{i}", response.body, "User #{user.id} was not able to view #{field.name} in CSV" + else + assert_not_include "Value#{i}", response.body, "User #{user.id} was able to view #{field.name} in CSV" + end + end + end + end + + def test_index_with_partial_custom_field_visibility + Issue.delete_all + p1 = Project.generate! + p2 = Project.generate! + user = User.generate! + User.add_to_project(user, p1, Role.find_all_by_id(1,3)) + User.add_to_project(user, p2, Role.find_all_by_id(3)) + Issue.generate!(:project => p1, :tracker_id => 1, :custom_field_values => {@field2.id => 'ValueA'}) + Issue.generate!(:project => p2, :tracker_id => 1, :custom_field_values => {@field2.id => 'ValueB'}) + Issue.generate!(:project => p1, :tracker_id => 1, :custom_field_values => {@field2.id => 'ValueC'}) + + @request.session[:user_id] = user.id + get :index, :c => ["subject", "cf_#{@field2.id}"] + assert_select 'td', :text => 'ValueA' + assert_select 'td', :text => 'ValueB', :count => 0 + assert_select 'td', :text => 'ValueC' + + get :index, :sort => "cf_#{@field2.id}" + # ValueB is not visible to user and ignored while sorting + assert_equal %w(ValueB ValueA ValueC), assigns(:issues).map{|i| i.custom_field_value(@field2)} + + get :index, :set_filter => '1', "cf_#{@field2.id}" => '*' + assert_equal %w(ValueA ValueC), assigns(:issues).map{|i| i.custom_field_value(@field2)} + + CustomField.update_all(:field_format => 'list') + get :index, :group => "cf_#{@field2.id}" + assert_equal %w(ValueA ValueC), assigns(:issues).map{|i| i.custom_field_value(@field2)} + end + + def test_create_should_send_notifications_according_custom_fields_visibility + # anonymous user is never notified + users_to_test = @users_to_test.reject {|k,v| k.anonymous?} + + ActionMailer::Base.deliveries.clear + @request.session[:user_id] = 1 + with_settings :bcc_recipients => '1' do + assert_difference 'Issue.count' do + post :create, + :project_id => 1, + :issue => { + :tracker_id => 1, + :status_id => 1, + :subject => 'New issue', + :priority_id => 5, + :custom_field_values => {@field1.id.to_s => 'Value0', @field2.id.to_s => 'Value1', @field3.id.to_s => 'Value2'}, + :watcher_user_ids => users_to_test.keys.map(&:id) + } + assert_response 302 + end + end + assert_equal users_to_test.values.uniq.size, ActionMailer::Base.deliveries.size + # tests that each user receives 1 email with the custom fields he is allowed to see only + users_to_test.each do |user, fields| + mails = ActionMailer::Base.deliveries.select {|m| m.bcc.include? user.mail} + assert_equal 1, mails.size + mail = mails.first + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_mail_body_match "Value#{i}", mail, "User #{user.id} was not able to view #{field.name} in notification" + else + assert_mail_body_no_match "Value#{i}", mail, "User #{user.id} was able to view #{field.name} in notification" + end + end + end + end + + def test_update_should_send_notifications_according_custom_fields_visibility + # anonymous user is never notified + users_to_test = @users_to_test.reject {|k,v| k.anonymous?} + + users_to_test.keys.each do |user| + Watcher.create!(:user => user, :watchable => @issue) + end + ActionMailer::Base.deliveries.clear + @request.session[:user_id] = 1 + with_settings :bcc_recipients => '1' do + put :update, + :id => @issue.id, + :issue => { + :custom_field_values => {@field1.id.to_s => 'NewValue0', @field2.id.to_s => 'NewValue1', @field3.id.to_s => 'NewValue2'} + } + assert_response 302 + end + assert_equal users_to_test.values.uniq.size, ActionMailer::Base.deliveries.size + # tests that each user receives 1 email with the custom fields he is allowed to see only + users_to_test.each do |user, fields| + mails = ActionMailer::Base.deliveries.select {|m| m.bcc.include? user.mail} + assert_equal 1, mails.size + mail = mails.first + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_mail_body_match "Value#{i}", mail, "User #{user.id} was not able to view #{field.name} in notification" + else + assert_mail_body_no_match "Value#{i}", mail, "User #{user.id} was able to view #{field.name} in notification" + end + end + end + end + + def test_updating_hidden_custom_fields_only_should_not_notifiy_user + # anonymous user is never notified + users_to_test = @users_to_test.reject {|k,v| k.anonymous?} + + users_to_test.keys.each do |user| + Watcher.create!(:user => user, :watchable => @issue) + end + ActionMailer::Base.deliveries.clear + @request.session[:user_id] = 1 + with_settings :bcc_recipients => '1' do + put :update, + :id => @issue.id, + :issue => { + :custom_field_values => {@field2.id.to_s => 'NewValue1', @field3.id.to_s => 'NewValue2'} + } + assert_response 302 + end + users_to_test.each do |user, fields| + mails = ActionMailer::Base.deliveries.select {|m| m.bcc.include? user.mail} + if (fields & [@field2, @field3]).any? + assert_equal 1, mails.size, "User #{user.id} was not notified" + else + assert_equal 0, mails.size, "User #{user.id} was notified" + end + end + end +end diff --git a/test/functional/search_custom_fields_visibility_test.rb b/test/functional/search_custom_fields_visibility_test.rb new file mode 100644 index 000000000..9b88aec62 --- /dev/null +++ b/test/functional/search_custom_fields_visibility_test.rb @@ -0,0 +1,78 @@ +# Redmine - project management software +# Copyright (C) 2006-2013 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. + +require File.expand_path('../../test_helper', __FILE__) + +class SearchCustomFieldsVisibilityTest < ActionController::TestCase + tests SearchController + fixtures :projects, + :users, + :roles, + :members, + :member_roles, + :issue_statuses, + :trackers, + :projects_trackers, + :enabled_modules, + :enumerations, + :workflows + + def setup + field_attributes = {:field_format => 'string', :is_for_all => true, :is_filter => true, :searchable => true, :trackers => Tracker.all} + @fields = [] + @fields << (@field1 = IssueCustomField.create!(field_attributes.merge(:name => 'Field 1', :visible => true))) + @fields << (@field2 = IssueCustomField.create!(field_attributes.merge(:name => 'Field 2', :visible => false, :role_ids => [1, 2]))) + @fields << (@field3 = IssueCustomField.create!(field_attributes.merge(:name => 'Field 3', :visible => false, :role_ids => [1, 3]))) + @issue = Issue.generate!( + :author_id => 1, + :project_id => 1, + :tracker_id => 1, + :custom_field_values => {@field1.id => 'Value0', @field2.id => 'Value1', @field3.id => 'Value2'} + ) + + @user_with_role_on_other_project = User.generate! + User.add_to_project(@user_with_role_on_other_project, Project.find(2), Role.find(3)) + + @users_to_test = { + User.find(1) => [@field1, @field2, @field3], + User.find(3) => [@field1, @field2], + @user_with_role_on_other_project => [@field1], # should see field1 only on Project 1 + User.generate! => [@field1], + User.anonymous => [@field1] + } + + Member.where(:project_id => 1).each do |member| + member.destroy unless @users_to_test.keys.include?(member.principal) + end + end + + def test_search_should_search_visible_custom_fields_only + @users_to_test.each do |user, fields| + @request.session[:user_id] = user.id + @fields.each_with_index do |field, i| + get :index, :q => "value#{i}" + assert_response :success + # we should get a result only if the custom field is visible + if fields.include?(field) + assert_equal 1, assigns(:results).size + else + assert_equal 0, assigns(:results).size + end + end + end + end +end diff --git a/test/functional/timelog_custom_fields_visibility_test.rb b/test/functional/timelog_custom_fields_visibility_test.rb new file mode 100644 index 000000000..c90eadc06 --- /dev/null +++ b/test/functional/timelog_custom_fields_visibility_test.rb @@ -0,0 +1,113 @@ +# Redmine - project management software +# Copyright (C) 2006-2013 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. + +require File.expand_path('../../test_helper', __FILE__) + +class TimelogCustomFieldsVisibilityTest < ActionController::TestCase + tests TimelogController + fixtures :projects, + :users, + :roles, + :members, + :member_roles, + :issue_statuses, + :trackers, + :projects_trackers, + :enabled_modules, + :enumerations, + :workflows + + def setup + field_attributes = {:field_format => 'string', :is_for_all => true, :is_filter => true, :trackers => Tracker.all} + @fields = [] + @fields << (@field1 = IssueCustomField.create!(field_attributes.merge(:name => 'Field 1', :visible => true))) + @fields << (@field2 = IssueCustomField.create!(field_attributes.merge(:name => 'Field 2', :visible => false, :role_ids => [1, 2]))) + @fields << (@field3 = IssueCustomField.create!(field_attributes.merge(:name => 'Field 3', :visible => false, :role_ids => [1, 3]))) + @issue = Issue.generate!( + :author_id => 1, + :project_id => 1, + :tracker_id => 1, + :custom_field_values => {@field1.id => 'Value0', @field2.id => 'Value1', @field3.id => 'Value2'} + ) + TimeEntry.generate!(:issue => @issue) + + @user_with_role_on_other_project = User.generate! + User.add_to_project(@user_with_role_on_other_project, Project.find(2), Role.find(3)) + + @users_to_test = { + User.find(1) => [@field1, @field2, @field3], + User.find(3) => [@field1, @field2], + @user_with_role_on_other_project => [@field1], # should see field1 only on Project 1 + User.generate! => [@field1], + User.anonymous => [@field1] + } + + Member.where(:project_id => 1).each do |member| + member.destroy unless @users_to_test.keys.include?(member.principal) + end + end + + def test_index_should_show_visible_custom_fields_only + @users_to_test.each do |user, fields| + @request.session[:user_id] = user.id + get :index, :project_id => 1, :issue_id => @issue.id, :c => (['hours'] + @fields.map{|f| "issue.cf_#{f.id}"}) + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_select 'td', {:text => "Value#{i}", :count => 1}, "User #{user.id} was not able to view #{field.name}" + else + assert_select 'td', {:text => "Value#{i}", :count => 0}, "User #{user.id} was able to view #{field.name}" + end + end + end + end + + def test_index_as_csv_should_show_visible_custom_fields_only + @users_to_test.each do |user, fields| + @request.session[:user_id] = user.id + get :index, :project_id => 1, :issue_id => @issue.id, :c => (['hours'] + @fields.map{|f| "issue.cf_#{f.id}"}), :format => 'csv' + @fields.each_with_index do |field, i| + if fields.include?(field) + assert_include "Value#{i}", response.body, "User #{user.id} was not able to view #{field.name} in CSV" + else + assert_not_include "Value#{i}", response.body, "User #{user.id} was able to view #{field.name} in CSV" + end + end + end + end + + def test_index_with_partial_custom_field_visibility_should_show_visible_custom_fields_only + Issue.delete_all + TimeEntry.delete_all + p1 = Project.generate! + p2 = Project.generate! + user = User.generate! + User.add_to_project(user, p1, Role.find_all_by_id(1,3)) + User.add_to_project(user, p2, Role.find_all_by_id(3)) + TimeEntry.generate!(:issue => Issue.generate!(:project => p1, :tracker_id => 1, :custom_field_values => {@field2.id => 'ValueA'})) + TimeEntry.generate!(:issue => Issue.generate!(:project => p2, :tracker_id => 1, :custom_field_values => {@field2.id => 'ValueB'})) + TimeEntry.generate!(:issue => Issue.generate!(:project => p1, :tracker_id => 1, :custom_field_values => {@field2.id => 'ValueC'})) + + @request.session[:user_id] = user.id + get :index, :c => ["hours", "issue.cf_#{@field2.id}"] + assert_select 'td', :text => 'ValueA' + assert_select 'td', :text => 'ValueB', :count => 0 + assert_select 'td', :text => 'ValueC' + + get :index, :set_filter => '1', "issue.cf_#{@field2.id}" => '*' + assert_equal %w(ValueA ValueC), assigns(:entries).map{|i| i.issue.custom_field_value(@field2)}.sort + end +end diff --git a/test/functional/workflows_controller_test.rb b/test/functional/workflows_controller_test.rb index f5bf3910b..001cb1a4e 100644 --- a/test/functional/workflows_controller_test.rb +++ b/test/functional/workflows_controller_test.rb @@ -200,6 +200,23 @@ class WorkflowsControllerTest < ActionController::TestCase end end + def test_get_permissions_should_disable_hidden_custom_fields + cf1 = IssueCustomField.generate!(:tracker_ids => [1], :visible => true) + cf2 = IssueCustomField.generate!(:tracker_ids => [1], :visible => false, :role_ids => [1]) + cf3 = IssueCustomField.generate!(:tracker_ids => [1], :visible => false, :role_ids => [1, 2]) + + get :permissions, :role_id => 2, :tracker_id => 1 + assert_response :success + assert_template 'permissions' + + assert_select 'select[name=?]:not(.disabled)', "permissions[#{cf1.id}][1]" + assert_select 'select[name=?]:not(.disabled)', "permissions[#{cf3.id}][1]" + + assert_select 'select[name=?][disabled=disabled]', "permissions[#{cf2.id}][1]" do + assert_select 'option[value=][selected=selected]', :text => 'Hidden' + end + end + def test_get_permissions_with_role_and_tracker_and_all_statuses WorkflowTransition.delete_all diff --git a/test/test_helper.rb b/test/test_helper.rb index 39826bc2d..725b1f596 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -169,8 +169,8 @@ class ActiveSupport::TestCase assert s.include?(expected), (message || "\"#{expected}\" not found in \"#{s}\"") end - def assert_not_include(expected, s) - assert !s.include?(expected), "\"#{expected}\" found in \"#{s}\"" + def assert_not_include(expected, s, message=nil) + assert !s.include?(expected), (message || "\"#{expected}\" found in \"#{s}\"") end def assert_select_in(text, *args, &block) @@ -178,19 +178,19 @@ class ActiveSupport::TestCase assert_select(d, *args, &block) end - def assert_mail_body_match(expected, mail) + def assert_mail_body_match(expected, mail, message=nil) if expected.is_a?(String) - assert_include expected, mail_body(mail) + assert_include expected, mail_body(mail), message else - assert_match expected, mail_body(mail) + assert_match expected, mail_body(mail), message end end - def assert_mail_body_no_match(expected, mail) + def assert_mail_body_no_match(expected, mail, message=nil) if expected.is_a?(String) - assert_not_include expected, mail_body(mail) + assert_not_include expected, mail_body(mail), message else - assert_no_match expected, mail_body(mail) + assert_no_match expected, mail_body(mail), message end end diff --git a/test/unit/custom_field_test.rb b/test/unit/custom_field_test.rb index 17a0041c0..051853abc 100644 --- a/test/unit/custom_field_test.rb +++ b/test/unit/custom_field_test.rb @@ -241,4 +241,42 @@ class CustomFieldTest < ActiveSupport::TestCase field = CustomField.find(1) assert_equal 'PostgreSQL', field.value_from_keyword('postgresql', Issue.find(1)) end + + def test_visibile_scope_with_admin_should_return_all_custom_fields + CustomField.delete_all + fields = [ + CustomField.generate!(:visible => true), + CustomField.generate!(:visible => false), + CustomField.generate!(:visible => false, :role_ids => [1, 3]), + CustomField.generate!(:visible => false, :role_ids => [1, 2]), + ] + + assert_equal 4, CustomField.visible(User.find(1)).count + end + + def test_visibile_scope_with_non_admin_user_should_return_visible_custom_fields + CustomField.delete_all + fields = [ + CustomField.generate!(:visible => true), + CustomField.generate!(:visible => false), + CustomField.generate!(:visible => false, :role_ids => [1, 3]), + CustomField.generate!(:visible => false, :role_ids => [1, 2]), + ] + user = User.generate! + User.add_to_project(user, Project.first, Role.find(3)) + + assert_equal [fields[0], fields[2]], CustomField.visible(user).order("id").to_a + end + + def test_visibile_scope_with_anonymous_user_should_return_visible_custom_fields + CustomField.delete_all + fields = [ + CustomField.generate!(:visible => true), + CustomField.generate!(:visible => false), + CustomField.generate!(:visible => false, :role_ids => [1, 3]), + CustomField.generate!(:visible => false, :role_ids => [1, 2]), + ] + + assert_equal [fields[0]], CustomField.visible(User.anonymous).order("id").to_a + end end diff --git a/test/unit/issue_custom_field_test.rb b/test/unit/issue_custom_field_test.rb new file mode 100644 index 000000000..26cc84467 --- /dev/null +++ b/test/unit/issue_custom_field_test.rb @@ -0,0 +1,42 @@ +# Redmine - project management software +# Copyright (C) 2006-2013 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. + +require File.expand_path('../../test_helper', __FILE__) + +class IssueCustomFieldTest < ActiveSupport::TestCase + include Redmine::I18n + + fixtures :roles + + def test_custom_field_with_visible_set_to_false_should_validate_roles + set_language_if_valid 'en' + field = IssueCustomField.new(:name => 'Field', :field_format => 'string', :visible => false) + assert !field.save + assert_include "Roles can't be blank", field.errors.full_messages + field.role_ids = [1, 2] + assert field.save + end + + def test_changing_visible_to_true_should_clear_roles + field = IssueCustomField.create!(:name => 'Field', :field_format => 'string', :visible => false, :role_ids => [1, 2]) + assert_equal 2, field.roles.count + + field.visible = true + field.save! + assert_equal 0, field.roles.count + end +end diff --git a/test/unit/lib/redmine/hook_test.rb b/test/unit/lib/redmine/hook_test.rb index 9f81b912d..f5ee1179f 100644 --- a/test/unit/lib/redmine/hook_test.rb +++ b/test/unit/lib/redmine/hook_test.rb @@ -154,14 +154,14 @@ class Redmine::Hook::ManagerTest < ActionView::TestCase issue = Issue.find(1) ActionMailer::Base.deliveries.clear - Mailer.issue_add(issue).deliver + Mailer.deliver_issue_add(issue) mail = ActionMailer::Base.deliveries.last @hook_module.add_listener(TestLinkToHook) hook_helper.call_hook(:view_layouts_base_html_head) ActionMailer::Base.deliveries.clear - Mailer.issue_add(issue).deliver + Mailer.deliver_issue_add(issue) mail2 = ActionMailer::Base.deliveries.last assert_equal mail_body(mail), mail_body(mail2) diff --git a/test/unit/mailer_test.rb b/test/unit/mailer_test.rb index c4891678f..93f4567d3 100644 --- a/test/unit/mailer_test.rb +++ b/test/unit/mailer_test.rb @@ -42,7 +42,7 @@ class MailerTest < ActiveSupport::TestCase Setting.protocol = 'https' journal = Journal.find(3) - assert Mailer.issue_edit(journal).deliver + assert Mailer.deliver_issue_edit(journal) mail = last_email assert_not_nil mail @@ -81,7 +81,7 @@ class MailerTest < ActiveSupport::TestCase Setting.protocol = 'http' journal = Journal.find(3) - assert Mailer.issue_edit(journal).deliver + assert Mailer.deliver_issue_edit(journal) mail = last_email assert_not_nil mail @@ -121,7 +121,7 @@ class MailerTest < ActiveSupport::TestCase Redmine::Utils.relative_url_root = nil journal = Journal.find(3) - assert Mailer.issue_edit(journal).deliver + assert Mailer.deliver_issue_edit(journal) mail = last_email assert_not_nil mail @@ -158,7 +158,7 @@ class MailerTest < ActiveSupport::TestCase def test_email_headers issue = Issue.find(1) - Mailer.issue_add(issue).deliver + Mailer.deliver_issue_add(issue) mail = last_email assert_not_nil mail assert_equal 'OOF', mail.header['X-Auto-Response-Suppress'].to_s @@ -168,7 +168,7 @@ class MailerTest < ActiveSupport::TestCase def test_email_headers_should_include_sender issue = Issue.find(1) - Mailer.issue_add(issue).deliver + Mailer.deliver_issue_add(issue) mail = last_email assert_equal issue.author.login, mail.header['X-Redmine-Sender'].to_s end @@ -176,7 +176,7 @@ class MailerTest < ActiveSupport::TestCase def test_plain_text_mail Setting.plain_text_mail = 1 journal = Journal.find(2) - Mailer.issue_edit(journal).deliver + Mailer.deliver_issue_edit(journal) mail = last_email assert_equal "text/plain; charset=UTF-8", mail.content_type assert_equal 0, mail.parts.size @@ -186,7 +186,7 @@ class MailerTest < ActiveSupport::TestCase def test_html_mail Setting.plain_text_mail = 0 journal = Journal.find(2) - Mailer.issue_edit(journal).deliver + Mailer.deliver_issue_edit(journal) mail = last_email assert_equal 2, mail.parts.size assert mail.encoded.include?('href') @@ -231,19 +231,21 @@ class MailerTest < ActiveSupport::TestCase end def test_issue_add_message_id - issue = Issue.find(1) - Mailer.issue_add(issue).deliver + issue = Issue.find(2) + Mailer.deliver_issue_add(issue) mail = last_email - assert_equal Mailer.message_id_for(issue), mail.message_id - assert_nil mail.references + assert_match /^redmine\.issue-2\.20060719190421\.[a-f0-9]+@example\.net/, mail.message_id + assert_include "redmine.issue-2.20060719190421@example.net", mail.references end def test_issue_edit_message_id - journal = Journal.find(1) - Mailer.issue_edit(journal).deliver + journal = Journal.find(3) + journal.issue = Issue.find(2) + + Mailer.deliver_issue_edit(journal) mail = last_email - assert_equal Mailer.message_id_for(journal), mail.message_id - assert_include Mailer.message_id_for(journal.issue), mail.references + assert_match /^redmine\.journal-3\.\d+\.[a-f0-9]+@example\.net/, mail.message_id + assert_include "redmine.issue-2.20060719190421@example.net", mail.references assert_select_email do # link to the update assert_select "a[href=?]", @@ -255,8 +257,8 @@ class MailerTest < ActiveSupport::TestCase message = Message.find(1) Mailer.message_posted(message).deliver mail = last_email - assert_equal Mailer.message_id_for(message), mail.message_id - assert_nil mail.references + assert_match /^redmine\.message-1\.\d+\.[a-f0-9]+@example\.net/, mail.message_id + assert_include "redmine.message-1.20070512151532@example.net", mail.references assert_select_email do # link to the message assert_select "a[href=?]", @@ -269,8 +271,8 @@ class MailerTest < ActiveSupport::TestCase message = Message.find(3) Mailer.message_posted(message).deliver mail = last_email - assert_equal Mailer.message_id_for(message), mail.message_id - assert_include Mailer.message_id_for(message.parent), mail.references + assert_match /^redmine\.message-3\.\d+\.[a-f0-9]+@example\.net/, mail.message_id + assert_include "redmine.message-1.20070512151532@example.net", mail.references assert_select_email do # link to the reply assert_select "a[href=?]", @@ -281,14 +283,14 @@ class MailerTest < ActiveSupport::TestCase test "#issue_add should notify project members" do issue = Issue.find(1) - assert Mailer.issue_add(issue).deliver + assert Mailer.deliver_issue_add(issue) assert last_email.bcc.include?('dlopper@somenet.foo') end test "#issue_add should not notify project members that are not allow to view the issue" do issue = Issue.find(1) Role.find(2).remove_permission!(:view_issues) - assert Mailer.issue_add(issue).deliver + assert Mailer.deliver_issue_add(issue) assert !last_email.bcc.include?('dlopper@somenet.foo') end @@ -302,7 +304,7 @@ class MailerTest < ActiveSupport::TestCase user.save Watcher.create!(:watchable => issue, :user => user) - assert Mailer.issue_add(issue).deliver + assert Mailer.deliver_issue_add(issue) assert last_email.bcc.include?(user.mail) end @@ -311,7 +313,7 @@ class MailerTest < ActiveSupport::TestCase user = User.find(9) Watcher.create!(:watchable => issue, :user => user) Role.non_member.remove_permission!(:view_issues) - assert Mailer.issue_add(issue).deliver + assert Mailer.deliver_issue_add(issue) assert !last_email.bcc.include?(user.mail) end @@ -320,7 +322,7 @@ class MailerTest < ActiveSupport::TestCase issue = Issue.find(1) valid_languages.each do |lang| Setting.default_language = lang.to_s - assert Mailer.issue_add(issue).deliver + assert Mailer.deliver_issue_add(issue) end end @@ -328,7 +330,7 @@ class MailerTest < ActiveSupport::TestCase journal = Journal.find(1) valid_languages.each do |lang| Setting.default_language = lang.to_s - assert Mailer.issue_edit(journal).deliver + assert Mailer.deliver_issue_edit(journal) end end @@ -338,11 +340,11 @@ class MailerTest < ActiveSupport::TestCase journal.save! Role.find(2).add_permission! :view_private_notes - Mailer.issue_edit(journal).deliver + Mailer.deliver_issue_edit(journal) assert_equal %w(dlopper@somenet.foo jsmith@somenet.foo), ActionMailer::Base.deliveries.last.bcc.sort Role.find(2).remove_permission! :view_private_notes - Mailer.issue_edit(journal).deliver + Mailer.deliver_issue_edit(journal) assert_equal %w(jsmith@somenet.foo), ActionMailer::Base.deliveries.last.bcc.sort end @@ -353,11 +355,11 @@ class MailerTest < ActiveSupport::TestCase journal.save! Role.non_member.add_permission! :view_private_notes - Mailer.issue_edit(journal).deliver + Mailer.deliver_issue_edit(journal) assert_include 'someone@foo.bar', ActionMailer::Base.deliveries.last.bcc.sort Role.non_member.remove_permission! :view_private_notes - Mailer.issue_edit(journal).deliver + Mailer.deliver_issue_edit(journal) assert_not_include 'someone@foo.bar', ActionMailer::Base.deliveries.last.bcc.sort end @@ -367,7 +369,7 @@ class MailerTest < ActiveSupport::TestCase journal.save! with_settings :default_language => 'en' do - Mailer.issue_edit(journal).deliver + Mailer.deliver_issue_edit(journal) end assert_mail_body_match '(Private notes)', last_email end diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 15f1cf21c..4ec430cf7 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -1201,6 +1201,28 @@ class QueryTest < ActiveSupport::TestCase assert ! query.available_filters["assigned_to_role"][:values].include?(['Anonymous','5']) end + def test_available_filters_should_include_custom_field_according_to_user_visibility + visible_field = IssueCustomField.generate!(:is_for_all => true, :is_filter => true, :visible => true) + hidden_field = IssueCustomField.generate!(:is_for_all => true, :is_filter => true, :visible => false, :role_ids => [1]) + + with_current_user User.find(3) do + query = IssueQuery.new + assert_include "cf_#{visible_field.id}", query.available_filters.keys + assert_not_include "cf_#{hidden_field.id}", query.available_filters.keys + end + end + + def test_available_columns_should_include_custom_field_according_to_user_visibility + visible_field = IssueCustomField.generate!(:is_for_all => true, :is_filter => true, :visible => true) + hidden_field = IssueCustomField.generate!(:is_for_all => true, :is_filter => true, :visible => false, :role_ids => [1]) + + with_current_user User.find(3) do + query = IssueQuery.new + assert_include :"cf_#{visible_field.id}", query.available_columns.map(&:name) + assert_not_include :"cf_#{hidden_field.id}", query.available_columns.map(&:name) + end + end + context "#statement" do context "with 'member_of_group' filter" do setup do -- 2.39.5