From 3f0a7fa15d6ec5d6fc7d3d594a31eab0419c3d4b Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Fri, 13 Sep 2013 14:29:24 +0200 Subject: [PATCH 01/21] Ability to assign issues to groups (#2964). Option is disabled by default. It can be turned on in application settings. Conflicts: app/controllers/reports_controller.rb app/models/issue.rb app/models/issue_category.rb app/models/mail_handler.rb app/models/project.rb app/views/issue_categories/_form.rhtml app/views/settings/_issues.rhtml config/locales/bg.yml config/locales/bs.yml config/locales/ca.yml config/locales/cs.yml config/locales/da.yml config/locales/de.yml config/locales/el.yml config/locales/en-GB.yml config/locales/en.yml config/locales/es.yml config/locales/eu.yml config/locales/fa.yml config/locales/fi.yml config/locales/fr.yml config/locales/gl.yml config/locales/he.yml config/locales/hr.yml config/locales/hu.yml config/locales/id.yml config/locales/it.yml config/locales/ja.yml config/locales/ko.yml config/locales/lt.yml config/locales/lv.yml config/locales/mk.yml config/locales/mn.yml config/locales/nl.yml config/locales/no.yml config/locales/pl.yml config/locales/pt-BR.yml config/locales/pt.yml config/locales/ro.yml config/locales/ru.yml config/locales/sk.yml config/locales/sl.yml config/locales/sr-YU.yml config/locales/sr.yml config/locales/sv.yml config/locales/th.yml config/locales/tr.yml config/locales/uk.yml config/locales/vi.yml config/locales/zh-TW.yml config/locales/zh.yml test/functional/issues_controller_test.rb test/unit/issue_category_test.rb Conflicts: app/models/issue.rb app/models/user.rb config/locales/de.yml test/functional/issues_controller_test.rb test/unit/issue_test.rb --- app/models/group.rb | 13 +++++++ app/models/issue_category.rb | 2 +- app/models/mail_handler.rb | 4 ++ app/models/principal.rb | 1 + app/models/project.rb | 27 +++++++++---- app/models/query.rb | 20 +++++++--- app/models/user.rb | 17 +++++++-- app/models/work_package.rb | 38 +++++++++++++++++-- app/views/issue_categories/_form.html.erb | 4 +- app/views/settings/_issues.html.erb | 2 + config/locales/de.yml | 4 +- config/locales/en.yml | 1 + config/settings.yml | 2 + .../work_package_action_mailer_spec.rb | 13 +++++++ spec/models/work_package_spec.rb | 37 ++++++++++++++++++ test/functional/issues_controller_test.rb | 16 ++++++++ test/unit/group_test.rb | 10 +++++ test/unit/issue_category_test.rb | 13 +++++++ test/unit/mail_handler_test.rb | 12 ++++++ 19 files changed, 212 insertions(+), 24 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index e27323230b..f24621b1f0 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -39,10 +39,14 @@ class Group < Principal validates_uniqueness_of :lastname, :case_sensitive => false validates_length_of :lastname, :maximum => 30 + before_destroy :remove_references_before_destroy + def to_s lastname.to_s end + alias :name :to_s + def user_added(user) members.each do |member| next if member.project.nil? @@ -85,4 +89,13 @@ class Group < Principal def add_member!(users) self.users << users end + + private + + # Removes references that are not handled by associations + def remove_references_before_destroy + return if self.id.nil? + + Issue.update_all 'assigned_to_id = NULL', ['assigned_to_id = ?', id] + end end diff --git a/app/models/issue_category.rb b/app/models/issue_category.rb index 2df37fc7ba..79b1ec28c2 100644 --- a/app/models/issue_category.rb +++ b/app/models/issue_category.rb @@ -30,7 +30,7 @@ class IssueCategory < ActiveRecord::Base include Redmine::SafeAttributes belongs_to :project - belongs_to :assigned_to, :class_name => 'User', :foreign_key => 'assigned_to_id' + belongs_to :assigned_to, :class_name => 'Principal', :foreign_key => 'assigned_to_id' has_many :work_packages, :foreign_key => 'category_id', :dependent => :nullify attr_protected :project_id diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb index 70b113565d..1ea632c240 100644 --- a/app/models/mail_handler.rb +++ b/app/models/mail_handler.rb @@ -425,9 +425,13 @@ class MailHandler < ActionMailer::Base a.lastname.to_s.downcase == lastname } end + if assignee.nil? + assignee ||= assignable.detect {|a| a.is_a?(Group) && a.name.downcase == keyword} + end if assignee.nil? assignee ||= assignable.detect {|a| a.name.downcase == keyword} end + assignee end end diff --git a/app/models/principal.rb b/app/models/principal.rb index 6cb0ed6163..38e1616fc6 100644 --- a/app/models/principal.rb +++ b/app/models/principal.rb @@ -35,6 +35,7 @@ class Principal < ActiveRecord::Base has_many :members, :foreign_key => 'user_id', :dependent => :destroy has_many :memberships, :class_name => 'Member', :foreign_key => 'user_id', :include => [ :project, :roles ], :conditions => "#{Project.table_name}.status=#{Project::STATUS_ACTIVE}", :order => "#{Project.table_name}.name" has_many :projects, :through => :memberships + has_many :issue_categories, :foreign_key => 'assigned_to_id', :dependent => :nullify # Groups and active users scope :active, :conditions => "#{Principal.table_name}.type='Group' OR (#{Principal.table_name}.type='User' AND #{Principal.table_name}.status = 1)" diff --git a/app/models/project.rb b/app/models/project.rb index 49b9475d1e..10bec1a25f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -45,11 +45,9 @@ class Project < ActiveRecord::Base has_many :members, :include => [:user, :roles], :conditions => "#{User.table_name}.type='User' AND #{User.table_name}.status=#{User::STATUSES[:active]}" has_many :assignable_members, :class_name => 'Member', - :include => [:user, :roles], - :conditions => ["#{User.table_name}.type=? AND #{User.table_name}.status=? AND roles.assignable = ?", - 'User', - User::STATUSES[:active], - true] + :include => [:principal, :roles], + :conditions => '#{ self.class.assignable_members_condition }' + has_many :memberships, :class_name => 'Member' has_many :member_principals, :class_name => 'Member', :include => :principal, @@ -596,9 +594,9 @@ class Project < ActiveRecord::Base Member.delete_all(['project_id = ?', id]) end - # Users issues can be assigned to + # Users/groups issues can be assigned to def assignable_users - assignable_members.map(&:user).sort + assignable_members.map(&:principal).compact.sort end # Returns the mail adresses of users that should be always notified on project events @@ -1109,4 +1107,19 @@ class Project < ActiveRecord::Base end update_attribute :status, STATUS_ARCHIVED end + + protected + + def self.assignable_members_condition + + condition = Setting.issue_group_assignment? ? + ["(#{Principal.table_name}.type=? OR #{Principal.table_name}.type=?)", 'User', 'Group'] : + ["(#{Principal.table_name}.type=?)", 'User'] + + condition[0] += " AND #{User.table_name}.status=? AND roles.assignable = ?" + condition << User::STATUSES[:active] + condition << true + + sanitize_sql_array condition + end end diff --git a/app/models/query.rb b/app/models/query.rb index 47243e99b6..b7a7f2dc32 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -144,15 +144,14 @@ class Query < ActiveRecord::Base "estimated_hours" => { :type => :integer, :order => 13 }, "done_ratio" => { :type => :integer, :order => 14 }} - user_values = [] - user_values << ["<< #{l(:label_me)} >>", "me"] if User.current.logged? + principals = [] if project - user_values += project.users.sort.collect{|s| [s.name, s.id.to_s] } + principals += project.principals.sort else all_projects = Project.visible.all if all_projects.any? # members of visible projects - user_values += User.active.find(:all, :conditions => ["#{User.table_name}.id IN (SELECT DISTINCT user_id FROM members WHERE project_id IN (?))", all_projects.collect(&:id)]).sort.collect{|s| [s.name, s.id.to_s] } + principals += Principal.active.find(:all, :conditions => ["#{User.table_name}.id IN (SELECT DISTINCT user_id FROM members WHERE project_id IN (?))", all_projects.collect(&:id)]).sort # project filter project_values = [] @@ -163,8 +162,17 @@ class Query < ActiveRecord::Base @available_filters["project_id"] = { :type => :list, :order => 1, :values => project_values} unless project_values.empty? end end - @available_filters["assigned_to_id"] = { :type => :list_optional, :order => 4, :values => user_values } unless user_values.empty? - @available_filters["author_id"] = { :type => :list, :order => 5, :values => user_values } unless user_values.empty? + users = principals.select {|p| p.is_a?(User)} + + assigned_to_values = [] + assigned_to_values << ["<< #{l(:label_me)} >>", "me"] if User.current.logged? + assigned_to_values += (Setting.issue_group_assignment? ? principals : users).collect{|s| [s.name, s.id.to_s] } + @available_filters["assigned_to_id"] = { :type => :list_optional, :order => 4, :values => assigned_to_values } unless assigned_to_values.empty? + + author_values = [] + author_values << ["<< #{l(:label_me)} >>", "me"] if User.current.logged? + author_values += users.collect{|s| [s.name, s.id.to_s] } + @available_filters["author_id"] = { :type => :list, :order => 5, :values => author_values } unless author_values.empty? group_values = Group.all.collect {|g| [g.name, g.id.to_s] } @available_filters["member_of_group"] = { :type => :list_optional, :order => 6, :values => group_values, :name => I18n.t('query_fields.member_of_group') } unless group_values.empty? diff --git a/app/models/user.rb b/app/models/user.rb index 8daf690dd5..8990a8362b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -566,6 +566,17 @@ class User < Principal @projects_by_role end + # Returns true if user is arg or belongs to arg + def is_or_belongs_to?(arg) + if arg.is_a?(User) + self == arg + elsif arg.is_a?(Group) + arg.users.include?(self) + else + false + end + end + # Return true if the user is allowed to do the specified action on a specific context # Action can be: # * a parameter-like Hash (eg. :controller => '/projects', :action => 'edit') @@ -657,7 +668,7 @@ class User < Principal true when 'selected' # user receives notifications for created/assigned issues on unselected projects - if object.is_a?(WorkPackage) && (object.author == self || object.assigned_to == self) + if object.is_a?(WorkPackage) && (object.author == self || is_or_belongs_to?(object.assigned_to)) true else false @@ -665,13 +676,13 @@ class User < Principal when 'none' false when 'only_my_events' - if object.is_a?(WorkPackage) && (object.author == self || object.assigned_to == self) + if object.is_a?(WorkPackage) && (object.author == self || is_or_belongs_to?(object.assigned_to)) true else false end when 'only_assigned' - if object.is_a?(WorkPackage) && object.assigned_to == self + if object.is_a?(WorkPackage) && is_or_belongs_to?(object.assigned_to) true else false diff --git a/app/models/work_package.rb b/app/models/work_package.rb index 57842df0ea..bfd9fc77cf 100644 --- a/app/models/work_package.rb +++ b/app/models/work_package.rb @@ -47,7 +47,7 @@ class WorkPackage < ActiveRecord::Base belongs_to :type belongs_to :status, :class_name => 'IssueStatus', :foreign_key => 'status_id' belongs_to :author, :class_name => 'User', :foreign_key => 'author_id' - belongs_to :assigned_to, :class_name => 'User', :foreign_key => 'assigned_to_id' + belongs_to :assigned_to, :class_name => 'Principal', :foreign_key => 'assigned_to_id' belongs_to :responsible, :class_name => "User", :foreign_key => "responsible_id" belongs_to :fixed_version, :class_name => 'Version', :foreign_key => 'fixed_version_id' belongs_to :priority, :class_name => 'IssuePriority', :foreign_key => 'priority_id' @@ -229,7 +229,20 @@ class WorkPackage < ActiveRecord::Base # Returns a SQL conditions string used to find all work units visible by the specified user def self.visible_condition(user, options={}) - Project.allowed_to_condition(user, :view_work_packages, options) + Project.allowed_to_condition(user, :view_work_packages, options) do |role, user| + case role.issues_visibility + when 'all' + nil + when 'default' + user_ids = [user.id] + user.groups.map(&:id) + "(#{table_name}.is_private = #{connection.quoted_false} OR #{table_name}.author_id = #{user.id} OR #{table_name}.assigned_to_id IN (#{user_ids}))" + when 'own' + user_ids = [user.id] + user.groups.map(&:id) + "(#{table_name}.author_id = #{user.id} OR #{table_name}.assigned_to_id IN (#{user_ids}))" + else + '1=0' + end + end end def self.done_ratio_disabled? @@ -246,7 +259,18 @@ class WorkPackage < ActiveRecord::Base # Returns true if usr or current user is allowed to view the work_package def visible?(usr=nil) - (usr || User.current).allowed_to?(:view_work_packages, self.project) + (usr || User.current).allowed_to?(:view_work_packages, self.project) do |role, user| + case role.issues_visibility + when 'all' + true + when 'default' + !self.is_private? || self.author == user || user.is_or_belongs_to?(assigned_to) + when 'own' + self.author == user || user.is_or_belongs_to?(assigned_to) + else + false + end + end end def copy_from(arg, options = {}) @@ -467,7 +491,13 @@ class WorkPackage < ActiveRecord::Base # Author and assignee are always notified unless they have been # locked or don't want to be notified notified << author if author && author.active? && author.notify_about?(self) - notified << assigned_to if assigned_to && assigned_to.active? && assigned_to.notify_about?(self) + if assigned_to + if assigned_to.is_a?(Group) + notified += assigned_to.users.select {|u| u.active? && u.notify_about?(self)} + else + notified << assigned_to if assigned_to.active? && assigned_to.notify_about?(self) + end + end notified.uniq! # Remove users that can not view the issue notified.reject! {|user| !visible?(user)} diff --git a/app/views/issue_categories/_form.html.erb b/app/views/issue_categories/_form.html.erb index 3aeed68e84..5cd78b75dd 100644 --- a/app/views/issue_categories/_form.html.erb +++ b/app/views/issue_categories/_form.html.erb @@ -31,5 +31,7 @@ See doc/COPYRIGHT.rdoc for more details.

<%= f.text_field :name, :size => 30, :required => true %>

-

<%= f.select :assigned_to_id, @project.users.sort.collect{|u| [u.name, u.id]}, :include_blank => true %>

+

+ <%= f.select :assigned_to_id, @project.assignable_users.sort.collect{|u| [u.name, u.id]}, :include_blank => true %> +

diff --git a/app/views/settings/_issues.html.erb b/app/views/settings/_issues.html.erb index 776d3d241b..6a49c35006 100644 --- a/app/views/settings/_issues.html.erb +++ b/app/views/settings/_issues.html.erb @@ -32,6 +32,8 @@ See doc/COPYRIGHT.rdoc for more details.

<%= setting_check_box :cross_project_work_package_relations %>

+

<%= setting_check_box :issue_group_assignment %>

+

<%= setting_check_box :display_subprojects_work_packages %>

<%= setting_check_box :work_package_startdate_is_adddate %>

diff --git a/config/locales/de.yml b/config/locales/de.yml index 6dad2949f1..af597debcc 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -1219,6 +1219,7 @@ de: setting_users_deletable_by_self: "Nutzer können ihren Account löschen" setting_welcome_text: "Willkommenstext" setting_wiki_compression: "Wiki-Historie komprimieren" + setting_issue_group_assignment: Ticketzuweisung zu Gruppen erlauben settings: general: "Allgemein" @@ -1393,7 +1394,7 @@ de: parent: "Zeige Unterprojekte von" planning_element_filters: "Planungselemente filtern" planning_element_responsible: "Planungselemente von diesem Planungsverantwortlichen anzeigen" - planning_element_types: "Typ anzeigen" + planning_element_types: "Planungselement-Typ anzeigen" project_time_filter: "Projekte mit Planungselementen eines Typs in einem Zeitfenster" project_time_filter_historical: "von %{startdate} bis %{enddate}" @@ -1521,7 +1522,6 @@ de: version_status_closed: "abgeschlossen" version_status_locked: "gesperrt" version_status_open: "offen" - warning_attachments_not_saved: "%{count} Datei(en) konnten nicht gespeichert werden." wiki_menu_item: "Menüpunkt" diff --git a/config/locales/en.yml b/config/locales/en.yml index 07dd9aefe7..5e9a95d5e0 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1195,6 +1195,7 @@ en: setting_users_deletable_by_self: "Users allowed to delete their accounts" setting_welcome_text: "Welcome text" setting_wiki_compression: "Wiki history compression" + setting_issue_group_assignment: Allow issue assignment to groups settings: general: "General" diff --git a/config/settings.yml b/config/settings.yml index a154ac8e40..71b016c997 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -162,6 +162,8 @@ user_format: format: symbol cross_project_work_package_relations: default: 0 +issue_group_assignment: + default: 0 notified_events: serialized: true default: diff --git a/spec/models/work_package/work_package_action_mailer_spec.rb b/spec/models/work_package/work_package_action_mailer_spec.rb index 9dba184e77..cdf3daf151 100644 --- a/spec/models/work_package/work_package_action_mailer_spec.rb +++ b/spec/models/work_package/work_package_action_mailer_spec.rb @@ -76,5 +76,18 @@ describe WorkPackage do it { should eq(0) } end + + context :group_assigned_work_package do + let(:group) { FactoryGirl.create(:group) } + + before do + group.users << user_1 + work_package.assigned_to = group + end + + subject { work_package.recipients } + + it { should include(user_1.mail) } + end end end diff --git a/spec/models/work_package_spec.rb b/spec/models/work_package_spec.rb index ac22a3ca91..9fdc69b9c5 100644 --- a/spec/models/work_package_spec.rb +++ b/spec/models/work_package_spec.rb @@ -95,6 +95,21 @@ describe WorkPackage do it { should be_nil } end end + + describe :assigned_to do + context :group_assignment do + let(:group) { FactoryGirl.create(:group) } + + before do + Setting.stub(:issue_group_assignment).and_return(true) + end + + subject { FactoryGirl.create(:work_package, + assigned_to: group).assigned_to } + + it { should eq(group) } + end + end end describe :type do @@ -165,6 +180,28 @@ describe WorkPackage do end end + context "with issue_group_assignment" do + let(:group) { FactoryGirl.create(:group) } + + before do + Setting.stub(:issue_group_assignment).and_return(true) + end + + subject { FactoryGirl.create(:work_package).assignable_users } + it { should include(group) } + end + + context "without issue_group_assignment" do + let(:group) { FactoryGirl.create(:group) } + + before do + Setting.stub(:issue_group_assignment).and_return(false) + end + + subject { FactoryGirl.create(:work_package).assignable_users } + it { should_not include(group) } + end + context "multiple users" do let(:user_2) { FactoryGirl.build_stubbed(:user) } diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 45cd755bee..13cf077543 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -119,6 +119,22 @@ class IssuesControllerTest < ActionController::TestCase assert_equal 0, ActionMailer::Base.deliveries.size end + def test_bulk_update_with_group_assignee + group = Group.find(11) + project = Project.find(1) + project.members << Member.new(:principal => group, :roles => [Role.first]) + + @request.session[:user_id] = 2 + # update issues assignee + post :bulk_update, :ids => [1, 2], :notes => 'Bulk editing', + :issue => {:priority_id => '', + :assigned_to_id => group.id, + :custom_field_values => {'2' => ''}} + + assert_response 302 + assert_equal [group, group], Issue.find_all_by_id([1, 2]).collect {|i| i.assigned_to} + end + def test_bulk_update_on_different_projects issue = WorkPackage.find(1) issue.recreate_initial_journal! diff --git a/test/unit/group_test.rb b/test/unit/group_test.rb index 1fb59ac984..c516e4d0e0 100644 --- a/test/unit/group_test.rb +++ b/test/unit/group_test.rb @@ -98,4 +98,14 @@ class GroupTest < ActiveSupport::TestCase @project.reload assert !@user.member_of?(@project) end + + def test_destroy_should_unassign_issues + group = Group.first + Issue.update_all(["assigned_to_id = ?", group.id], 'id = 1') + + assert group.destroy + assert group.destroyed? + + assert_equal nil, Issue.find(1).assigned_to_id + end end diff --git a/test/unit/issue_category_test.rb b/test/unit/issue_category_test.rb index 1e734abafe..0f91ed3ee6 100644 --- a/test/unit/issue_category_test.rb +++ b/test/unit/issue_category_test.rb @@ -38,6 +38,19 @@ class IssueCategoryTest < ActiveSupport::TestCase assert_equal @category.work_packages, [@issue] end + def test_create + assert IssueCategory.new(:project_id => 2, :name => 'New category').save + category = IssueCategory.first(:order => 'id DESC') + assert_equal 'New category', category.name + end + + def test_create_with_group_assignment + assert IssueCategory.new(:project_id => 2, :name => 'Group assignment', :assigned_to_id => 11).save + category = IssueCategory.first(:order => 'id DESC') + assert_kind_of Group, category.assigned_to + assert_equal Group.find(11), category.assigned_to + end + # Make sure the category was nullified on the issue def test_destroy @category.destroy diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb index 346352dcc8..1fcae50a09 100644 --- a/test/unit/mail_handler_test.rb +++ b/test/unit/mail_handler_test.rb @@ -103,6 +103,18 @@ class MailHandlerTest < ActiveSupport::TestCase assert issue.description.include?('Lorem ipsum dolor sit amet, consectetuer adipiscing elit.') end + def test_add_issue_with_group_assignment + with_settings :issue_group_assignment => '1' do + issue = submit_email('ticket_on_given_project.eml') do |email| + email.gsub!('Assigned to: John Smith', 'Assigned to: B Team') + end + assert issue.is_a?(Issue) + assert !issue.new_record? + issue.reload + assert_equal Group.find(11), issue.assigned_to + end + end + def test_add_issue_with_partial_attributes_override issue = submit_email('ticket_with_attributes.eml', :issue => {:priority => 'High'}, :allow_override => ['type']) assert issue.is_a?(WorkPackage) From 2c17ec8d501612d52a9c6943576ab763d939b3b3 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Fri, 13 Sep 2013 14:33:11 +0200 Subject: [PATCH 02/21] Include issues asigned to user's groups when using 'assigned to me' filter (#2964). Conflicts: app/views/my/blocks/_issuesassignedtome.rhtml Conflicts: app/views/my/blocks/_issuesassignedtome.html.erb --- app/models/query.rb | 9 ++++++++- .../my/blocks/_issuesassignedtome.html.erb | 4 +++- test/unit/query_test.rb | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/app/models/query.rb b/app/models/query.rb index b7a7f2dc32..87964bcc01 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -417,7 +417,14 @@ class Query < ActiveRecord::Base # "me" value subsitution if %w(assigned_to_id author_id watcher_id).include?(field) - v.push(User.current.logged? ? User.current.id.to_s : "0") if v.delete("me") + if v.delete("me") + if User.current.logged? + v.push(User.current.id.to_s) + v += User.current.group_ids.map(&:to_s) if field == 'assigned_to_id' + else + v.push("0") + end + end end sql = '' diff --git a/app/views/my/blocks/_issuesassignedtome.html.erb b/app/views/my/blocks/_issuesassignedtome.html.erb index 91bb55cce8..2ee9cf7657 100644 --- a/app/views/my/blocks/_issuesassignedtome.html.erb +++ b/app/views/my/blocks/_issuesassignedtome.html.erb @@ -33,7 +33,9 @@ See doc/COPYRIGHT.rdoc for more details. :include => [ :status, :project, :type, :priority ], :order => "#{IssuePriority.table_name}.position DESC, #{WorkPackage.table_name}.updated_at DESC") %> -<% assigned_count = WorkPackage.visible.open.count(:conditions => {:assigned_to_id => User.current.id}) %> +<% assigned_count = WorkPackage.visible.open.count(:conditions => { + :assigned_to_id => [User.current.id] + User.current.group_ids + }) %>

<%=l(:label_assigned_to_me_work_packages)%> (<%= assigned_count %>)

diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index d2d683cd85..07988e2135 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -209,6 +209,24 @@ class QueryTest < ActiveSupport::TestCase find_issues_with_query(query) end + def test_filter_assigned_to_me + user = User.find(2) + group = Group.find(10) + User.current = user + i1 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => user) + i2 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => group) + i3 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => Group.find(11)) + group.users << user + + query = Query.new(:name => '_', :filters => { 'assigned_to_id' => {:operator => '=', :values => ['me']}}) + result = query.issues + assert_equal Issue.visible.all(:conditions => {:assigned_to_id => ([2] + user.reload.group_ids)}).sort_by(&:id), result.sort_by(&:id) + + assert result.include?(i1) + assert result.include?(i2) + assert !result.include?(i3) + end + def test_filter_watched_issues User.current = User.find(1) query = Query.new(:name => '_', :filters => { 'watcher_id' => {:operator => '=', :values => ['me']}}) From 1b9338c24c9dec01501b36dec7926e0179b2366c Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Sat, 6 Oct 2012 21:03:49 +0200 Subject: [PATCH 03/21] renames variable invalidated by cherry-picking commit 578fdc62f26c23951b2d2c2b9be0040c7ade0634 --- app/models/query.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/query.rb b/app/models/query.rb index 87964bcc01..7bc6868cfa 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -183,7 +183,7 @@ class Query < ActiveRecord::Base if User.current.logged? # populate the watcher list with the same user list as other user filters if the user has the :view_work_package_watchers permission in at least one project # TODO: this could be differentiated more, e.g. all users could watch issues in public projects, but won't necessarily be shown here - watcher_values = User.current.allowed_to_globally?(:view_work_package_watchers, {}) ? user_values : [["<< #{l(:label_me)} >>", "me"]] + watcher_values = User.current.allowed_to_globally?(:view_work_package_watchers, {}) ? principals : [["<< #{l(:label_me)} >>", "me"]] @available_filters["watcher_id"] = { :type => :list, :order => 15, :values => watcher_values } end From 999ed55d6439d087b09bfadb080e4a62fe12ed3f Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 9 Oct 2012 19:12:22 +0200 Subject: [PATCH 04/21] fixes filtering by watchers broken by 9f18636 --- app/models/query.rb | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/app/models/query.rb b/app/models/query.rb index 7bc6868cfa..7272fc26bf 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -162,16 +162,23 @@ class Query < ActiveRecord::Base @available_filters["project_id"] = { :type => :list, :order => 1, :values => project_values} unless project_values.empty? end end - users = principals.select {|p| p.is_a?(User)} + principals_by_class = principals.group_by(&:class) - assigned_to_values = [] - assigned_to_values << ["<< #{l(:label_me)} >>", "me"] if User.current.logged? - assigned_to_values += (Setting.issue_group_assignment? ? principals : users).collect{|s| [s.name, s.id.to_s] } + user_values = principals_by_class[User].present? ? + principals_by_class[User].collect{ |s| [s.name, s.id.to_s] }.sort : + [] + + group_values = Setting.issue_group_assignment? && principals_by_class[Group].present? ? + principals_by_class[Group].collect{ |s| [s.name, s.id.to_s] }.sort : + [] + + assigned_to_values = (user_values + group_values).sort + assigned_to_values = [["<< #{l(:label_me)} >>", "me"]] + assigned_to_values if User.current.logged? @available_filters["assigned_to_id"] = { :type => :list_optional, :order => 4, :values => assigned_to_values } unless assigned_to_values.empty? author_values = [] author_values << ["<< #{l(:label_me)} >>", "me"] if User.current.logged? - author_values += users.collect{|s| [s.name, s.id.to_s] } + author_values += user_values @available_filters["author_id"] = { :type => :list, :order => 5, :values => author_values } unless author_values.empty? group_values = Group.all.collect {|g| [g.name, g.id.to_s] } @@ -183,7 +190,8 @@ class Query < ActiveRecord::Base if User.current.logged? # populate the watcher list with the same user list as other user filters if the user has the :view_work_package_watchers permission in at least one project # TODO: this could be differentiated more, e.g. all users could watch issues in public projects, but won't necessarily be shown here - watcher_values = User.current.allowed_to_globally?(:view_work_package_watchers, {}) ? principals : [["<< #{l(:label_me)} >>", "me"]] + watcher_values = [["<< #{l(:label_me)} >>", "me"]] + watcher_values << user_values if User.current.allowed_to_globally?(:view_work_package_watchers, {}) @available_filters["watcher_id"] = { :type => :list, :order => 15, :values => watcher_values } end From 1e694eefe4a30b3fc6d9cc3c5d23be347edb8ba7 Mon Sep 17 00:00:00 2001 From: jwollert Date: Fri, 13 Sep 2013 14:36:24 +0200 Subject: [PATCH 05/21] fixin' oldschool validations and broken tests Conflicts: app/models/project.rb Conflicts: test/unit/issue_test.rb --- app/models/issue_category.rb | 2 +- app/models/project.rb | 3 +-- test/unit/group_test.rb | 13 ++++++++----- test/unit/issue_category_test.rb | 21 ++++++++++++++------- test/unit/mail_handler_test.rb | 4 ++-- 5 files changed, 26 insertions(+), 17 deletions(-) diff --git a/app/models/issue_category.rb b/app/models/issue_category.rb index 79b1ec28c2..7ea6027990 100644 --- a/app/models/issue_category.rb +++ b/app/models/issue_category.rb @@ -42,7 +42,7 @@ class IssueCategory < ActiveRecord::Base # validates that assignee is member of the issue category's project validates_each :assigned_to_id do |record, attr, value| if value # allow nil - record.errors.add(attr, l(:error_must_be_project_member)) unless record.project.user_ids.include?(value.to_i) + record.errors.add(attr, l(:error_must_be_project_member)) unless record.project.principals.map(&:id).include? value end end diff --git a/app/models/project.rb b/app/models/project.rb index 10bec1a25f..0b4a61fad1 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -46,8 +46,7 @@ class Project < ActiveRecord::Base has_many :assignable_members, :class_name => 'Member', :include => [:principal, :roles], - :conditions => '#{ self.class.assignable_members_condition }' - + :conditions => Proc.new { self.class.assignable_members_condition } has_many :memberships, :class_name => 'Member' has_many :member_principals, :class_name => 'Member', :include => :principal, diff --git a/test/unit/group_test.rb b/test/unit/group_test.rb index c516e4d0e0..194ad39939 100644 --- a/test/unit/group_test.rb +++ b/test/unit/group_test.rb @@ -34,6 +34,7 @@ class GroupTest < ActiveSupport::TestCase super @group = FactoryGirl.create :group @member = FactoryGirl.build :member + @issue = FactoryGirl.create :issue @roles = FactoryGirl.create_list :role, 2 @member.force_attributes = { :principal => @group, :role_ids => @roles.map(&:id) } @member.save! @@ -100,12 +101,14 @@ class GroupTest < ActiveSupport::TestCase end def test_destroy_should_unassign_issues - group = Group.first - Issue.update_all(["assigned_to_id = ?", group.id], 'id = 1') + @issue.assigned_to_id = @group.id - assert group.destroy - assert group.destroyed? + assert @issue.save + assert @issue.assigned_to_id == @group.id + assert @group.destroy + assert @group.destroyed? - assert_equal nil, Issue.find(1).assigned_to_id + @issue.reload + assert_equal nil, @issue.assigned_to_id end end diff --git a/test/unit/issue_category_test.rb b/test/unit/issue_category_test.rb index 0f91ed3ee6..eb16d4cf54 100644 --- a/test/unit/issue_category_test.rb +++ b/test/unit/issue_category_test.rb @@ -39,16 +39,23 @@ class IssueCategoryTest < ActiveSupport::TestCase end def test_create - assert IssueCategory.new(:project_id => 2, :name => 'New category').save - category = IssueCategory.first(:order => 'id DESC') - assert_equal 'New category', category.name + (new_cat = IssueCategory.new).force_attributes = {:project_id => @project.id, :name => 'New category'} + assert new_cat.valid? + assert new_cat.save + assert_equal 'New category', new_cat.name end def test_create_with_group_assignment - assert IssueCategory.new(:project_id => 2, :name => 'Group assignment', :assigned_to_id => 11).save - category = IssueCategory.first(:order => 'id DESC') - assert_kind_of Group, category.assigned_to - assert_equal Group.find(11), category.assigned_to + group = FactoryGirl.create :group + role = FactoryGirl.create :role + (Member.new.tap do |m| + m.force_attributes = { :principal => group, :project => @project, :role_ids => [role.id] } + end).save! + (new_cat = IssueCategory.new).force_attributes = {:project_id => @project.id, :name => 'Group assignment', :assigned_to_id => group.id} + assert new_cat.valid? + assert new_cat.save + assert_kind_of Group, new_cat.assigned_to + assert_equal group, new_cat.assigned_to end # Make sure the category was nullified on the issue diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb index 1fcae50a09..cb872dc7a8 100644 --- a/test/unit/mail_handler_test.rb +++ b/test/unit/mail_handler_test.rb @@ -171,10 +171,10 @@ class MailHandlerTest < ActiveSupport::TestCase end def test_add_issue_should_match_assignee_on_display_name # added from redmine - not sure if it is ok here - user = User.generate!(:firstname => 'Foo Bar', :lastname => 'Foo Baz') + user = User.generate!(:firstname => 'Foo', :lastname => 'Bar') User.add_to_project(user, Project.find(2), Role.generate!(:name => 'Superhero')) issue = submit_email('ticket_on_given_project.eml') do |email| - email.sub!(/^Assigned to.*$/, 'Assigned to: Foo Bar Foo baz') + email.sub!(/^Assigned to.*$/, 'Assigned to: Foo Bar') end assert issue.is_a?(WorkPackage) assert_equal user, issue.assigned_to From 8c64cf97f171b1a8579b4d76bc0d89c71a25dc71 Mon Sep 17 00:00:00 2001 From: jwollert Date: Fri, 13 Sep 2013 14:37:03 +0200 Subject: [PATCH 06/21] reflect name change of tracker to type Conflicts: test/unit/issue_test.rb --- test/unit/query_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 07988e2135..d7ddea5989 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -213,9 +213,9 @@ class QueryTest < ActiveSupport::TestCase user = User.find(2) group = Group.find(10) User.current = user - i1 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => user) - i2 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => group) - i3 = Issue.generate!(:project_id => 1, :tracker_id => 1, :assigned_to => Group.find(11)) + i1 = Issue.generate!(:project_id => 1, :type_id => 1, :assigned_to => user) + i2 = Issue.generate!(:project_id => 1, :type_id => 1, :assigned_to => group) + i3 = Issue.generate!(:project_id => 1, :type_id => 1, :assigned_to => Group.find(11)) group.users << user query = Query.new(:name => '_', :filters => { 'assigned_to_id' => {:operator => '=', :values => ['me']}}) From 00663548fdd824febf84bdb9f855746e8c995ffd Mon Sep 17 00:00:00 2001 From: jwollert Date: Thu, 15 Aug 2013 11:26:43 +0200 Subject: [PATCH 07/21] fixes queries watcher values --- app/models/query.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/query.rb b/app/models/query.rb index 7272fc26bf..e7b49ddcbf 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -191,7 +191,7 @@ class Query < ActiveRecord::Base # populate the watcher list with the same user list as other user filters if the user has the :view_work_package_watchers permission in at least one project # TODO: this could be differentiated more, e.g. all users could watch issues in public projects, but won't necessarily be shown here watcher_values = [["<< #{l(:label_me)} >>", "me"]] - watcher_values << user_values if User.current.allowed_to_globally?(:view_work_package_watchers, {}) + user_values.each { |v| watcher_values << v } if User.current.allowed_to_globally?(:view_work_packages_watchers, {}) @available_filters["watcher_id"] = { :type => :list, :order => 15, :values => watcher_values } end From cbc75dce3bf2caca19bf93e0d7a1c885c20c8c9a Mon Sep 17 00:00:00 2001 From: jwollert Date: Fri, 13 Sep 2013 14:47:09 +0200 Subject: [PATCH 08/21] fixes wrong setting in workpackage group spec, test still fails tho --- spec/models/work_package_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/work_package_spec.rb b/spec/models/work_package_spec.rb index 9fdc69b9c5..f9f877b61a 100644 --- a/spec/models/work_package_spec.rb +++ b/spec/models/work_package_spec.rb @@ -184,7 +184,7 @@ describe WorkPackage do let(:group) { FactoryGirl.create(:group) } before do - Setting.stub(:issue_group_assignment).and_return(true) + Setting.stub(:issue_group_assignment?).and_return(true) end subject { FactoryGirl.create(:work_package).assignable_users } @@ -195,7 +195,7 @@ describe WorkPackage do let(:group) { FactoryGirl.create(:group) } before do - Setting.stub(:issue_group_assignment).and_return(false) + Setting.stub(:issue_group_assignment?).and_return(false) end subject { FactoryGirl.create(:work_package).assignable_users } From 57eb8992b0acf9a886b6bdb27385b505eba87ee6 Mon Sep 17 00:00:00 2001 From: jwollert Date: Fri, 13 Sep 2013 15:10:43 +0200 Subject: [PATCH 09/21] group needs to be a member to the project in order to be assignable --- spec/models/work_package_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/models/work_package_spec.rb b/spec/models/work_package_spec.rb index f9f877b61a..5f30d0db9d 100644 --- a/spec/models/work_package_spec.rb +++ b/spec/models/work_package_spec.rb @@ -182,23 +182,27 @@ describe WorkPackage do context "with issue_group_assignment" do let(:group) { FactoryGirl.create(:group) } + let(:work_package) { FactoryGirl.create(:work_package) } before do Setting.stub(:issue_group_assignment?).and_return(true) + work_package.project.add_member! group, FactoryGirl.create(:role) end - subject { FactoryGirl.create(:work_package).assignable_users } + subject { work_package.assignable_users } it { should include(group) } end context "without issue_group_assignment" do let(:group) { FactoryGirl.create(:group) } + let(:work_package) { FactoryGirl.create(:work_package) } before do Setting.stub(:issue_group_assignment?).and_return(false) + work_package.project.add_member! group, FactoryGirl.create(:role) end - subject { FactoryGirl.create(:work_package).assignable_users } + subject { work_package.assignable_users } it { should_not include(group) } end From 23c3fa429121d9f9768f2ed00c5251e4cdf522a6 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 13 Sep 2013 15:40:29 +0200 Subject: [PATCH 10/21] replace Issue with WorkPackage --- app/models/group.rb | 2 +- test/functional/issues_controller_test.rb | 2 +- test/unit/group_test.rb | 12 ++++++------ test/unit/mail_handler_test.rb | 14 +++++++------- test/unit/query_test.rb | 8 ++++---- 5 files changed, 19 insertions(+), 19 deletions(-) diff --git a/app/models/group.rb b/app/models/group.rb index f24621b1f0..3c73327a48 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -96,6 +96,6 @@ class Group < Principal def remove_references_before_destroy return if self.id.nil? - Issue.update_all 'assigned_to_id = NULL', ['assigned_to_id = ?', id] + WorkPackage.update_all 'assigned_to_id = NULL', ['assigned_to_id = ?', id] end end diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb index 13cf077543..bc7233e497 100644 --- a/test/functional/issues_controller_test.rb +++ b/test/functional/issues_controller_test.rb @@ -132,7 +132,7 @@ class IssuesControllerTest < ActionController::TestCase :custom_field_values => {'2' => ''}} assert_response 302 - assert_equal [group, group], Issue.find_all_by_id([1, 2]).collect {|i| i.assigned_to} + assert_equal [group, group], WorkPackage.find_all_by_id([1, 2]).collect {|i| i.assigned_to} end def test_bulk_update_on_different_projects diff --git a/test/unit/group_test.rb b/test/unit/group_test.rb index 194ad39939..10c3b53d18 100644 --- a/test/unit/group_test.rb +++ b/test/unit/group_test.rb @@ -34,7 +34,7 @@ class GroupTest < ActiveSupport::TestCase super @group = FactoryGirl.create :group @member = FactoryGirl.build :member - @issue = FactoryGirl.create :issue + @work_package = FactoryGirl.create :work_package @roles = FactoryGirl.create_list :role, 2 @member.force_attributes = { :principal => @group, :role_ids => @roles.map(&:id) } @member.save! @@ -101,14 +101,14 @@ class GroupTest < ActiveSupport::TestCase end def test_destroy_should_unassign_issues - @issue.assigned_to_id = @group.id + @work_package.assigned_to_id = @group.id - assert @issue.save - assert @issue.assigned_to_id == @group.id + assert @work_package.save + assert @work_package.assigned_to_id == @group.id assert @group.destroy assert @group.destroyed? - @issue.reload - assert_equal nil, @issue.assigned_to_id + @work_package.reload + assert_equal nil, @work_package.assigned_to_id end end diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb index cb872dc7a8..a1578a49cc 100644 --- a/test/unit/mail_handler_test.rb +++ b/test/unit/mail_handler_test.rb @@ -103,15 +103,15 @@ class MailHandlerTest < ActiveSupport::TestCase assert issue.description.include?('Lorem ipsum dolor sit amet, consectetuer adipiscing elit.') end - def test_add_issue_with_group_assignment - with_settings :issue_group_assignment => '1' do - issue = submit_email('ticket_on_given_project.eml') do |email| + def test_add_work_package_with_group_assignment + with_settings :work_package_group_assignment => '1' do + work_package = submit_email('ticket_on_given_project.eml') do |email| email.gsub!('Assigned to: John Smith', 'Assigned to: B Team') end - assert issue.is_a?(Issue) - assert !issue.new_record? - issue.reload - assert_equal Group.find(11), issue.assigned_to + assert work_package.is_a?(Issue) + assert !work_package.new_record? + work_package.reload + assert_equal Group.find(11), work_package.assigned_to end end diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index d7ddea5989..46c476453b 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -213,14 +213,14 @@ class QueryTest < ActiveSupport::TestCase user = User.find(2) group = Group.find(10) User.current = user - i1 = Issue.generate!(:project_id => 1, :type_id => 1, :assigned_to => user) - i2 = Issue.generate!(:project_id => 1, :type_id => 1, :assigned_to => group) - i3 = Issue.generate!(:project_id => 1, :type_id => 1, :assigned_to => Group.find(11)) + i1 = WorkPackage.generate!(:project_id => 1, :type_id => 1, :assigned_to => user) + i2 = WorkPackage.generate!(:project_id => 1, :type_id => 1, :assigned_to => group) + i3 = WorkPackage.generate!(:project_id => 1, :type_id => 1, :assigned_to => Group.find(11)) group.users << user query = Query.new(:name => '_', :filters => { 'assigned_to_id' => {:operator => '=', :values => ['me']}}) result = query.issues - assert_equal Issue.visible.all(:conditions => {:assigned_to_id => ([2] + user.reload.group_ids)}).sort_by(&:id), result.sort_by(&:id) + assert_equal WorkPackage.visible.all(:conditions => {:assigned_to_id => ([2] + user.reload.group_ids)}).sort_by(&:id), result.sort_by(&:id) assert result.include?(i1) assert result.include?(i2) From abfa9e14657d172f83031aaf09ea85c6534f6791 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 13 Sep 2013 15:41:40 +0200 Subject: [PATCH 11/21] renames setting --- app/models/project.rb | 2 +- app/models/query.rb | 2 +- app/views/settings/_issues.html.erb | 2 +- config/locales/de.yml | 2 +- config/locales/en.yml | 2 +- config/settings.yml | 2 +- spec/models/work_package_spec.rb | 20 ++++++++++---------- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 0b4a61fad1..c4bc0a5963 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1111,7 +1111,7 @@ class Project < ActiveRecord::Base def self.assignable_members_condition - condition = Setting.issue_group_assignment? ? + condition = Setting.work_package_group_assignment? ? ["(#{Principal.table_name}.type=? OR #{Principal.table_name}.type=?)", 'User', 'Group'] : ["(#{Principal.table_name}.type=?)", 'User'] diff --git a/app/models/query.rb b/app/models/query.rb index e7b49ddcbf..1f6f8d9ddc 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -168,7 +168,7 @@ class Query < ActiveRecord::Base principals_by_class[User].collect{ |s| [s.name, s.id.to_s] }.sort : [] - group_values = Setting.issue_group_assignment? && principals_by_class[Group].present? ? + group_values = Setting.work_package_group_assignment? && principals_by_class[Group].present? ? principals_by_class[Group].collect{ |s| [s.name, s.id.to_s] }.sort : [] diff --git a/app/views/settings/_issues.html.erb b/app/views/settings/_issues.html.erb index 6a49c35006..981ca9f20d 100644 --- a/app/views/settings/_issues.html.erb +++ b/app/views/settings/_issues.html.erb @@ -32,7 +32,7 @@ See doc/COPYRIGHT.rdoc for more details.

<%= setting_check_box :cross_project_work_package_relations %>

-

<%= setting_check_box :issue_group_assignment %>

+

<%= setting_check_box :work_package_group_assignment %>

<%= setting_check_box :display_subprojects_work_packages %>

diff --git a/config/locales/de.yml b/config/locales/de.yml index af597debcc..7142bbdf65 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -1219,7 +1219,7 @@ de: setting_users_deletable_by_self: "Nutzer können ihren Account löschen" setting_welcome_text: "Willkommenstext" setting_wiki_compression: "Wiki-Historie komprimieren" - setting_issue_group_assignment: Ticketzuweisung zu Gruppen erlauben + setting_work_package_group_assignment: "Zuweisung zu Gruppen erlauben" settings: general: "Allgemein" diff --git a/config/locales/en.yml b/config/locales/en.yml index 5e9a95d5e0..a9ae8fa48d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1195,7 +1195,7 @@ en: setting_users_deletable_by_self: "Users allowed to delete their accounts" setting_welcome_text: "Welcome text" setting_wiki_compression: "Wiki history compression" - setting_issue_group_assignment: Allow issue assignment to groups + setting_work_package_group_assignment: "Allow assignment to groups" settings: general: "General" diff --git a/config/settings.yml b/config/settings.yml index 71b016c997..d14615cc41 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -162,7 +162,7 @@ user_format: format: symbol cross_project_work_package_relations: default: 0 -issue_group_assignment: +work_package_group_assignment: default: 0 notified_events: serialized: true diff --git a/spec/models/work_package_spec.rb b/spec/models/work_package_spec.rb index 5f30d0db9d..267061ebb0 100644 --- a/spec/models/work_package_spec.rb +++ b/spec/models/work_package_spec.rb @@ -101,7 +101,7 @@ describe WorkPackage do let(:group) { FactoryGirl.create(:group) } before do - Setting.stub(:issue_group_assignment).and_return(true) + Setting.stub(:work_package_group_assignment).and_return(true) end subject { FactoryGirl.create(:work_package, @@ -180,12 +180,12 @@ describe WorkPackage do end end - context "with issue_group_assignment" do + context "with work_package_group_assignment" do let(:group) { FactoryGirl.create(:group) } let(:work_package) { FactoryGirl.create(:work_package) } before do - Setting.stub(:issue_group_assignment?).and_return(true) + Setting.stub(:work_package_group_assignment?).and_return(true) work_package.project.add_member! group, FactoryGirl.create(:role) end @@ -193,12 +193,12 @@ describe WorkPackage do it { should include(group) } end - context "without issue_group_assignment" do + context "without work_package_group_assignment" do let(:group) { FactoryGirl.create(:group) } let(:work_package) { FactoryGirl.create(:work_package) } before do - Setting.stub(:issue_group_assignment?).and_return(false) + Setting.stub(:work_package_group_assignment?).and_return(false) work_package.project.add_member! group, FactoryGirl.create(:role) end @@ -343,7 +343,7 @@ describe WorkPackage do end shared_examples_for "save with open version" do - before do + before do work_package.status = status_open work_package.fixed_version = version_open end @@ -356,7 +356,7 @@ describe WorkPackage do context "in closed version" do include_context "in closed version" - before do + before do work_package.status = status_open work_package.save end @@ -414,13 +414,13 @@ describe WorkPackage do end context "time entry 1" do - subject { work_package.time_entries } + subject { work_package.time_entries } it { should include(time_entry_1) } end context "time entry 2" do - subject { work_package.time_entries } + subject { work_package.time_entries } it { should include(time_entry_2) } end @@ -455,7 +455,7 @@ describe WorkPackage do it { should eq(target_category.id) } end - + it_behaves_like "moved work package" end From 542e2f91b852b33950dfad57bf84c88c7b28b144 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 13 Sep 2013 15:42:09 +0200 Subject: [PATCH 12/21] removes duplicate statements (probably merge relic) --- app/models/mail_handler.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/models/mail_handler.rb b/app/models/mail_handler.rb index 1ea632c240..a6710ec3e9 100644 --- a/app/models/mail_handler.rb +++ b/app/models/mail_handler.rb @@ -428,9 +428,6 @@ class MailHandler < ActionMailer::Base if assignee.nil? assignee ||= assignable.detect {|a| a.is_a?(Group) && a.name.downcase == keyword} end - if assignee.nil? - assignee ||= assignable.detect {|a| a.name.downcase == keyword} - end assignee end From ffcacf9a17ccceda68a5ba0074eb021b4de2093e Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 13 Sep 2013 15:42:23 +0200 Subject: [PATCH 13/21] corrects comment --- app/models/project.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index c4bc0a5963..9138bafb08 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -593,7 +593,7 @@ class Project < ActiveRecord::Base Member.delete_all(['project_id = ?', id]) end - # Users/groups issues can be assigned to + # Users/groups a work_package can be assigned to def assignable_users assignable_members.map(&:principal).compact.sort end From 7f5b3d14d4778c4a0684a7c6a2100693c43b6aaa Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 13 Sep 2013 18:15:15 +0200 Subject: [PATCH 14/21] adapt tests to removal of issues --- test/unit/mail_handler_test.rb | 2 +- test/unit/query_test.rb | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb index a1578a49cc..ee908a4ac0 100644 --- a/test/unit/mail_handler_test.rb +++ b/test/unit/mail_handler_test.rb @@ -108,7 +108,7 @@ class MailHandlerTest < ActiveSupport::TestCase work_package = submit_email('ticket_on_given_project.eml') do |email| email.gsub!('Assigned to: John Smith', 'Assigned to: B Team') end - assert work_package.is_a?(Issue) + assert work_package.is_a?(WorkPackage) assert !work_package.new_record? work_package.reload assert_equal Group.find(11), work_package.assigned_to diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 46c476453b..565a9afa1d 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -212,19 +212,22 @@ class QueryTest < ActiveSupport::TestCase def test_filter_assigned_to_me user = User.find(2) group = Group.find(10) + project = Project.find(1) User.current = user - i1 = WorkPackage.generate!(:project_id => 1, :type_id => 1, :assigned_to => user) - i2 = WorkPackage.generate!(:project_id => 1, :type_id => 1, :assigned_to => group) - i3 = WorkPackage.generate!(:project_id => 1, :type_id => 1, :assigned_to => Group.find(11)) + i1 = FactoryGirl.create(:work_package, :project => project, :type => project.types.first, :assigned_to => user) + i2 = FactoryGirl.create(:work_package, :project => project, :type => project.types.first, :assigned_to => group) + i3 = FactoryGirl.create(:work_package, :project => project, :type => project.types.first, :assigned_to => Group.find(11)) group.users << user query = Query.new(:name => '_', :filters => { 'assigned_to_id' => {:operator => '=', :values => ['me']}}) - result = query.issues + result = query.results.work_packages assert_equal WorkPackage.visible.all(:conditions => {:assigned_to_id => ([2] + user.reload.group_ids)}).sort_by(&:id), result.sort_by(&:id) assert result.include?(i1) assert result.include?(i2) assert !result.include?(i3) + + User.current = nil end def test_filter_watched_issues From 0df5585e41b793b9c29d77bba5ac33f4de3e042f Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 13 Sep 2013 18:41:54 +0200 Subject: [PATCH 15/21] removes code accidentally added by cherry-picking --- app/models/work_package.rb | 28 ++-------------------------- 1 file changed, 2 insertions(+), 26 deletions(-) diff --git a/app/models/work_package.rb b/app/models/work_package.rb index bfd9fc77cf..824786a89b 100644 --- a/app/models/work_package.rb +++ b/app/models/work_package.rb @@ -229,20 +229,7 @@ class WorkPackage < ActiveRecord::Base # Returns a SQL conditions string used to find all work units visible by the specified user def self.visible_condition(user, options={}) - Project.allowed_to_condition(user, :view_work_packages, options) do |role, user| - case role.issues_visibility - when 'all' - nil - when 'default' - user_ids = [user.id] + user.groups.map(&:id) - "(#{table_name}.is_private = #{connection.quoted_false} OR #{table_name}.author_id = #{user.id} OR #{table_name}.assigned_to_id IN (#{user_ids}))" - when 'own' - user_ids = [user.id] + user.groups.map(&:id) - "(#{table_name}.author_id = #{user.id} OR #{table_name}.assigned_to_id IN (#{user_ids}))" - else - '1=0' - end - end + Project.allowed_to_condition(user, :view_work_packages, options) end def self.done_ratio_disabled? @@ -259,18 +246,7 @@ class WorkPackage < ActiveRecord::Base # Returns true if usr or current user is allowed to view the work_package def visible?(usr=nil) - (usr || User.current).allowed_to?(:view_work_packages, self.project) do |role, user| - case role.issues_visibility - when 'all' - true - when 'default' - !self.is_private? || self.author == user || user.is_or_belongs_to?(assigned_to) - when 'own' - self.author == user || user.is_or_belongs_to?(assigned_to) - else - false - end - end + (usr || User.current).allowed_to?(:view_work_packages, self.project) end def copy_from(arg, options = {}) From f8b3db3a7a2dccbe706e2728932e7ccc316a50a0 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 13 Sep 2013 20:42:31 +0200 Subject: [PATCH 16/21] whitespace fixes --- app/views/my/blocks/_issuesassignedtome.html.erb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/my/blocks/_issuesassignedtome.html.erb b/app/views/my/blocks/_issuesassignedtome.html.erb index 2ee9cf7657..7b63c25d5f 100644 --- a/app/views/my/blocks/_issuesassignedtome.html.erb +++ b/app/views/my/blocks/_issuesassignedtome.html.erb @@ -43,10 +43,10 @@ See doc/COPYRIGHT.rdoc for more details. <% if assigned.length > 0 %>

<%= link_to l(:label_work_package_view_all_assigned_to_me), controller: :work_packages, - action: :index, - set_filter: 1, - assigned_to_id: 'me', - sort: 'priority:desc,updated_at:desc' %>

+ action: :index, + set_filter: 1, + assigned_to_id: 'me', + sort: 'priority:desc,updated_at:desc' %>

<% end %> <% content_for :header_tags do %> From ef9cce611cba031b95455b68c68b2406a3be726e Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 13 Sep 2013 20:48:58 +0200 Subject: [PATCH 17/21] removes accidentally added localization --- config/locales/de.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/config/locales/de.yml b/config/locales/de.yml index 7142bbdf65..deee4f167e 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -1394,7 +1394,6 @@ de: parent: "Zeige Unterprojekte von" planning_element_filters: "Planungselemente filtern" planning_element_responsible: "Planungselemente von diesem Planungsverantwortlichen anzeigen" - planning_element_types: "Planungselement-Typ anzeigen" project_time_filter: "Projekte mit Planungselementen eines Typs in einem Zeitfenster" project_time_filter_historical: "von %{startdate} bis %{enddate}" From c5f330a191b8f76e9bce36607f868e1a11e5dcee Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Fri, 13 Sep 2013 20:50:58 +0200 Subject: [PATCH 18/21] adds changelog --- doc/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/CHANGELOG.md b/doc/CHANGELOG.md index 3b2ff87e42..f2e501b157 100644 --- a/doc/CHANGELOG.md +++ b/doc/CHANGELOG.md @@ -29,7 +29,8 @@ See doc/COPYRIGHT.rdoc for more details. # Changelog -* `#1770` New Comment Section layout errors +* `#1715` Group assigned work packages +* `#1770` New Comment Section layout errors ## 3.0.0pre17 From 7548e827d81983c299dfd6165c15d449076de09d Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 16 Sep 2013 12:56:04 +0200 Subject: [PATCH 19/21] Does not count number of assigned groups --- app/views/my/blocks/_issuesassignedtome.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/my/blocks/_issuesassignedtome.html.erb b/app/views/my/blocks/_issuesassignedtome.html.erb index 7b63c25d5f..b6b24be992 100644 --- a/app/views/my/blocks/_issuesassignedtome.html.erb +++ b/app/views/my/blocks/_issuesassignedtome.html.erb @@ -34,7 +34,7 @@ See doc/COPYRIGHT.rdoc for more details. :order => "#{IssuePriority.table_name}.position DESC, #{WorkPackage.table_name}.updated_at DESC") %> <% assigned_count = WorkPackage.visible.open.count(:conditions => { - :assigned_to_id => [User.current.id] + User.current.group_ids + :assigned_to_id => User.current.id }) %>

<%=l(:label_assigned_to_me_work_packages)%> (<%= assigned_count %>)

From 44dee8bb540322a039d4081edd6538c4671dbc30 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Mon, 23 Sep 2013 11:09:56 +0200 Subject: [PATCH 20/21] assign packages to deleted user upon group deletion --- app/models/group.rb | 8 +++- spec/models/group_spec.rb | 70 ++++++++++++++++++++++++++++ spec/support/shared/become_member.rb | 2 +- test/unit/group_test.rb | 12 ----- 4 files changed, 78 insertions(+), 14 deletions(-) create mode 100644 spec/models/group_spec.rb diff --git a/app/models/group.rb b/app/models/group.rb index 3c73327a48..df96a8b476 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -96,6 +96,12 @@ class Group < Principal def remove_references_before_destroy return if self.id.nil? - WorkPackage.update_all 'assigned_to_id = NULL', ['assigned_to_id = ?', id] + deleted_user = DeletedUser.first + + WorkPackage.update_all({ :assigned_to_id => deleted_user.id }, + { :assigned_to_id => id }) + + Journal::WorkPackageJournal.update_all({ :assigned_to_id => deleted_user.id }, + { :assigned_to_id => id }) end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb new file mode 100644 index 0000000000..dd6d530a2b --- /dev/null +++ b/spec/models/group_spec.rb @@ -0,0 +1,70 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2013 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# 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. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' +require_relative '../support/shared/become_member' + +describe Group do + include BecomeMember + + let(:group) { FactoryGirl.build(:group) } + let(:user) { FactoryGirl.build(:user) } + let(:project) { FactoryGirl.create(:project_with_types) } + let(:issue_status) { FactoryGirl.create(:issue_status) } + let(:package) { FactoryGirl.build(:work_package, :type => project.types.first, + :author => user, + :project => project, + :status => issue_status) } + + describe :destroy do + describe 'work packages assigned to the group' do + before do + become_member_with_permissions project, group, [:view_work_packages] + package.assigned_to = group + + package.save! + end + + it 'should reassign the work package to nobody' do + group.destroy + + package.reload + + package.assigned_to.should == DeletedUser.first + end + + it 'should update all journals to have the deleted user as assigned' do + group.destroy + + package.reload + + package.journals.all?{ |j| j.data.assigned_to_id == DeletedUser.first.id }.should be_true + end + end + end +end diff --git a/spec/support/shared/become_member.rb b/spec/support/shared/become_member.rb index 4cf3ff8a0d..cc667cc2ab 100644 --- a/spec/support/shared/become_member.rb +++ b/spec/support/shared/become_member.rb @@ -38,7 +38,7 @@ module BecomeMember role = FactoryGirl.create(:role, :permissions => permissions) - member = FactoryGirl.build(:member, :user => user, :project => project) + member = FactoryGirl.build(:member, :principal => user, :project => project) member.roles = [role] member.save! end diff --git a/test/unit/group_test.rb b/test/unit/group_test.rb index 10c3b53d18..5ef4d6f9e2 100644 --- a/test/unit/group_test.rb +++ b/test/unit/group_test.rb @@ -99,16 +99,4 @@ class GroupTest < ActiveSupport::TestCase @project.reload assert !@user.member_of?(@project) end - - def test_destroy_should_unassign_issues - @work_package.assigned_to_id = @group.id - - assert @work_package.save - assert @work_package.assigned_to_id == @group.id - assert @group.destroy - assert @group.destroyed? - - @work_package.reload - assert_equal nil, @work_package.assigned_to_id - end end From 168843148587e78514c00254fd62cea9233dd05d Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Mon, 23 Sep 2013 11:10:49 +0200 Subject: [PATCH 21/21] removes duplicate method, present also in UsersHelper --- app/helpers/groups_helper.rb | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 9a841cc63c..fc2119e580 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -28,19 +28,10 @@ #++ module GroupsHelper - # Options for the new membership projects combo-box - def options_for_membership_project_select(user, projects) - options = content_tag('option', "--- #{l(:actionview_instancetag_blank_option)} ---") - options << project_tree_options_for_select(projects) do |p| - {:disabled => (user.projects.include?(p))} - end - options - end - def group_settings_tabs - tabs = [{:name => 'general', :partial => 'groups/general', :label => :label_general}, - {:name => 'users', :partial => 'groups/users', :label => :label_user_plural}, - {:name => 'memberships', :partial => 'groups/memberships', :label => :label_project_plural} - ] + [{:name => 'general', :partial => 'groups/general', :label => :label_general}, + {:name => 'users', :partial => 'groups/users', :label => :label_user_plural}, + {:name => 'memberships', :partial => 'groups/memberships', :label => :label_project_plural} + ] end end