From: Jean-Philippe Lang Date: Thu, 19 Jan 2017 20:16:49 +0000 (+0000) Subject: Adds updated_by and last_updated_by filters on issues (#17720). X-Git-Tag: 3.4.0~383 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=151a215ea45c1870842de49d0640b1557f712498;p=redmine.git Adds updated_by and last_updated_by filters on issues (#17720). git-svn-id: http://svn.redmine.org/redmine/trunk@16228 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- diff --git a/app/models/issue_query.rb b/app/models/issue_query.rb index 53702b9ba..1b1477e3c 100644 --- a/app/models/issue_query.rb +++ b/app/models/issue_query.rb @@ -165,6 +165,14 @@ class IssueQuery < Query add_available_filter "issue_id", :type => :integer, :label => :label_issue + add_available_filter("updated_by", + :type => :list, :values => lambda { author_values } + ) + + add_available_filter("last_updated_by", + :type => :list, :values => lambda { author_values } + ) + Tracker.disabled_core_fields(trackers).each {|field| delete_available_filter field } @@ -341,6 +349,27 @@ class IssueQuery < Query raise StatementInvalid.new(e.message) end + def sql_for_updated_by_field(field, operator, value) + neg = (operator == '!' ? 'NOT' : '') + subquery = "SELECT 1 FROM #{Journal.table_name}" + + " WHERE #{Journal.table_name}.journalized_type='Issue' AND #{Journal.table_name}.journalized_id=#{Issue.table_name}.id" + + " AND (#{sql_for_field field, '=', value, Journal.table_name, 'user_id'})" + + " AND (#{Journal.visible_notes_condition(User.current, :skip_pre_condition => true)})" + + "#{neg} EXISTS (#{subquery})" + end + + def sql_for_last_updated_by_field(field, operator, value) + neg = (operator == '!' ? 'NOT' : '') + subquery = "SELECT 1 FROM #{Journal.table_name} sj" + + " WHERE sj.journalized_type='Issue' AND sj.journalized_id=#{Issue.table_name}.id AND (#{sql_for_field field, '=', value, 'sj', 'user_id'})" + + " AND sj.id = (SELECT MAX(#{Journal.table_name}.id) FROM #{Journal.table_name}" + + " WHERE #{Journal.table_name}.journalized_type='Issue' AND #{Journal.table_name}.journalized_id=#{Issue.table_name}.id" + + " AND (#{Journal.visible_notes_condition(User.current, :skip_pre_condition => true)}))" + + "#{neg} EXISTS (#{subquery})" + end + def sql_for_watcher_id_field(field, operator, value) db_table = Watcher.table_name "#{Issue.table_name}.id #{ operator == '=' ? 'IN' : 'NOT IN' } (SELECT #{db_table}.watchable_id FROM #{db_table} WHERE #{db_table}.watchable_type='Issue' AND " + diff --git a/app/models/journal.rb b/app/models/journal.rb index 447cbe4b5..7d1a6eb34 100644 --- a/app/models/journal.rb +++ b/app/models/journal.rb @@ -47,10 +47,11 @@ class Journal < ActiveRecord::Base scope :visible, lambda {|*args| user = args.shift || User.current - private_notes_condition = Project.allowed_to_condition(user, :view_private_notes, *args) + options = args.shift || {} + joins(:issue => :project). - where(Issue.visible_condition(user, *args)). - where("(#{Journal.table_name}.private_notes = ? OR #{Journal.table_name}.user_id = ? OR (#{private_notes_condition}))", false, user.id) + where(Issue.visible_condition(user, options)). + where(Journal.visible_notes_condition(user, :skip_pre_condition => true)) } safe_attributes 'notes', @@ -58,6 +59,12 @@ class Journal < ActiveRecord::Base safe_attributes 'private_notes', :if => lambda {|journal, user| user.allowed_to?(:set_notes_private, journal.project)} + # Returns a SQL condition to filter out journals with notes that are not visible to user + def self.visible_notes_condition(user=User.current, options={}) + private_notes_permission = Project.allowed_to_condition(user, :view_private_notes, options) + sanitize_sql_for_conditions(["(#{table_name}.private_notes = ? OR #{table_name}.user_id = ? OR (#{private_notes_permission}))", false, user.id]) + end + def initialize(*args) super if journalized diff --git a/app/models/project.rb b/app/models/project.rb index e5eb12c58..fb2ffc3a6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -173,13 +173,14 @@ class Project < ActiveRecord::Base # Returns a SQL conditions string used to find all projects for which +user+ has the given +permission+ # # Valid options: - # * :project => limit the condition to project - # * :with_subprojects => limit the condition to project and its subprojects - # * :member => limit the condition to the user projects + # * :skip_pre_condition => true don't check that the module is enabled (eg. when the condition is already set elsewhere in the query) + # * :project => project limit the condition to project + # * :with_subprojects => true limit the condition to project and its subprojects + # * :member => true limit the condition to the user projects def self.allowed_to_condition(user, permission, options={}) perm = Redmine::AccessControl.permission(permission) base_statement = (perm && perm.read? ? "#{Project.table_name}.status <> #{Project::STATUS_ARCHIVED}" : "#{Project.table_name}.status = #{Project::STATUS_ACTIVE}") - if perm && perm.project_module + if !options[:skip_pre_condition] && perm && perm.project_module # If the permission belongs to a project module, make sure the module is enabled base_statement << " AND #{Project.table_name}.id IN (SELECT em.project_id FROM #{EnabledModule.table_name} em WHERE em.name='#{perm.project_module}')" end diff --git a/app/models/query.rb b/app/models/query.rb index 7c85d7164..4f183329a 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -802,7 +802,7 @@ class Query < ActiveRecord::Base operator = operator_for(field) # "me" value substitution - if %w(assigned_to_id author_id user_id watcher_id).include?(field) + if %w(assigned_to_id author_id user_id watcher_id updated_by last_updated_by).include?(field) if v.delete("me") if User.current.logged? v.push(User.current.id.to_s) diff --git a/config/locales/en.yml b/config/locales/en.yml index 7ba059947..24d73e65d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -370,6 +370,8 @@ en: field_default_version: Default version field_remote_ip: IP address field_textarea_font: Font used for text areas + field_updated_by: Updated by + field_last_updated_by: Last updated by setting_app_title: Application title setting_app_subtitle: Application subtitle diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 73aabc0a9..6136c7978 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -382,6 +382,8 @@ fr: field_total_estimated_hours: Temps estimé total field_default_version: Version par défaut field_textarea_font: Police utilisée pour les champs texte + field_updated_by: Mise à jour par + field_last_updated_by: Dernière mise à jour par setting_app_title: Titre de l'application setting_app_subtitle: Sous-titre de l'application diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 8b88b11cd..81f62516d 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -719,6 +719,93 @@ class QueryTest < ActiveSupport::TestCase end end + def test_filter_updated_by + user = User.generate! + Journal.create!(:user_id => user.id, :journalized => Issue.find(2), :notes => 'Notes') + Journal.create!(:user_id => user.id, :journalized => Issue.find(3), :notes => 'Notes') + Journal.create!(:user_id => 2, :journalized => Issue.find(3), :notes => 'Notes') + + query = IssueQuery.new(:name => '_') + filter_name = "updated_by" + assert_include filter_name, query.available_filters.keys + + query.filters = {filter_name => {:operator => '=', :values => [user.id]}} + assert_equal [2, 3], find_issues_with_query(query).map(&:id).sort + + query.filters = {filter_name => {:operator => '!', :values => [user.id]}} + assert_equal (Issue.ids.sort - [2, 3]), find_issues_with_query(query).map(&:id).sort + end + + def test_filter_updated_by_should_ignore_private_notes_that_are_not_visible + user = User.generate! + Journal.create!(:user_id => user.id, :journalized => Issue.find(2), :notes => 'Notes', :private_notes => true) + Journal.create!(:user_id => user.id, :journalized => Issue.find(3), :notes => 'Notes') + + query = IssueQuery.new(:name => '_') + filter_name = "updated_by" + assert_include filter_name, query.available_filters.keys + + with_current_user User.anonymous do + query.filters = {filter_name => {:operator => '=', :values => [user.id]}} + assert_equal [3], find_issues_with_query(query).map(&:id).sort + end + end + + def test_filter_updated_by_me + user = User.generate! + Journal.create!(:user_id => user.id, :journalized => Issue.find(2), :notes => 'Notes') + + with_current_user user do + query = IssueQuery.new(:name => '_') + filter_name = "updated_by" + assert_include filter_name, query.available_filters.keys + + query.filters = {filter_name => {:operator => '=', :values => ['me']}} + assert_equal [2], find_issues_with_query(query).map(&:id).sort + end + end + + def test_filter_last_updated_by + user = User.generate! + Journal.create!(:user_id => user.id, :journalized => Issue.find(2), :notes => 'Notes') + Journal.create!(:user_id => user.id, :journalized => Issue.find(3), :notes => 'Notes') + Journal.create!(:user_id => 2, :journalized => Issue.find(3), :notes => 'Notes') + + query = IssueQuery.new(:name => '_') + filter_name = "last_updated_by" + assert_include filter_name, query.available_filters.keys + + query.filters = {filter_name => {:operator => '=', :values => [user.id]}} + assert_equal [2], find_issues_with_query(query).map(&:id).sort + end + + def test_filter_last_updated_by_should_ignore_private_notes_that_are_not_visible + user1 = User.generate! + user2 = User.generate! + Journal.create!(:user_id => user1.id, :journalized => Issue.find(2), :notes => 'Notes') + Journal.create!(:user_id => user2.id, :journalized => Issue.find(2), :notes => 'Notes', :private_notes => true) + + query = IssueQuery.new(:name => '_') + filter_name = "last_updated_by" + assert_include filter_name, query.available_filters.keys + + with_current_user User.anonymous do + query.filters = {filter_name => {:operator => '=', :values => [user1.id]}} + assert_equal [2], find_issues_with_query(query).map(&:id).sort + + query.filters = {filter_name => {:operator => '=', :values => [user2.id]}} + assert_equal [], find_issues_with_query(query).map(&:id).sort + end + + with_current_user User.find(2) do + query.filters = {filter_name => {:operator => '=', :values => [user1.id]}} + assert_equal [], find_issues_with_query(query).map(&:id).sort + + query.filters = {filter_name => {:operator => '=', :values => [user2.id]}} + assert_equal [2], find_issues_with_query(query).map(&:id).sort + end + end + def test_user_custom_field_filtered_on_me User.current = User.find(2) cf = IssueCustomField.create!(:field_format => 'user', :is_for_all => true, :is_filter => true, :name => 'User custom field', :tracker_ids => [1])