From 1f80a4b0d9eabbfe6c350abf60fb3548ef724aa3 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Lang Date: Sat, 29 Sep 2012 16:38:53 +0000 Subject: [PATCH] Merged r10433, r10437 from trunk. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/branches/2.1-stable@10526 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/models/issue.rb | 44 ++++++++++++++---------- app/models/role.rb | 5 +++ app/views/roles/_form.html.erb | 2 ++ test/functional/roles_controller_test.rb | 8 +++++ test/unit/issue_test.rb | 28 ++++++++------- 5 files changed, 56 insertions(+), 31 deletions(-) diff --git a/app/models/issue.rb b/app/models/issue.rb index f3851927e..6ef6518d3 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -84,17 +84,21 @@ class Issue < ActiveRecord::Base # Returns a SQL conditions string used to find all issues visible by the specified user def self.visible_condition(user, options={}) Project.allowed_to_condition(user, :view_issues, 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.join(',')}))" - 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.join(',')}))" + if user.logged? + 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.join(',')}))" + 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.join(',')}))" + else + '1=0' + end else - '1=0' + "(#{table_name}.is_private = #{connection.quoted_false})" end end end @@ -102,15 +106,19 @@ class Issue < ActiveRecord::Base # Returns true if usr or current user is allowed to view the issue def visible?(usr=nil) (usr || User.current).allowed_to?(:view_issues, 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) + if user.logged? + 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 else - false + !self.is_private? end end end diff --git a/app/models/role.rb b/app/models/role.rb index 5fd437648..15ed0e10d 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -133,6 +133,11 @@ class Role < ActiveRecord::Base self.builtin != 0 end + # Return true if the role is the anonymous role + def anonymous? + builtin == 2 + end + # Return true if the role is a project member role def member? !self.builtin? diff --git a/app/views/roles/_form.html.erb b/app/views/roles/_form.html.erb index 8ae0a604f..d028c2f1c 100644 --- a/app/views/roles/_form.html.erb +++ b/app/views/roles/_form.html.erb @@ -1,5 +1,6 @@ <%= error_messages_for 'role' %> +<% unless @role.anonymous? %>
<% unless @role.builtin? %>

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

@@ -11,6 +12,7 @@ <%= select_tag(:copy_workflow_from, content_tag("option") + options_from_collection_for_select(@roles, :id, :name, params[:copy_workflow_from] || @copy_from.try(:id))) %>

<% end %>
+<% end %>

<%= l(:label_permissions) %>

diff --git a/test/functional/roles_controller_test.rb b/test/functional/roles_controller_test.rb index 868c987a6..8aa74457a 100644 --- a/test/functional/roles_controller_test.rb +++ b/test/functional/roles_controller_test.rb @@ -110,6 +110,14 @@ class RolesControllerTest < ActionController::TestCase assert_response :success assert_template 'edit' assert_equal Role.find(1), assigns(:role) + assert_select 'select[name=?]', 'role[issues_visibility]' + end + + def test_edit_anonymous + get :edit, :id => Role.anonymous.id + assert_response :success + assert_template 'edit' + assert_select 'select[name=?]', 'role[issues_visibility]', 0 end def test_edit_invalid_should_respond_with_404 diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb index 05224701d..fc08313bc 100644 --- a/test/unit/issue_test.rb +++ b/test/unit/issue_test.rb @@ -25,7 +25,7 @@ class IssueTest < ActiveSupport::TestCase :versions, :issue_statuses, :issue_categories, :issue_relations, :workflows, :enumerations, - :issues, + :issues, :journals, :journal_details, :custom_fields, :custom_fields_projects, :custom_fields_trackers, :custom_values, :time_entries @@ -105,18 +105,6 @@ class IssueTest < ActiveSupport::TestCase assert_visibility_match User.anonymous, issues end - def test_visible_scope_for_anonymous_with_own_issues_visibility - Role.anonymous.update_attribute :issues_visibility, 'own' - Issue.create!(:project_id => 1, :tracker_id => 1, - :author_id => User.anonymous.id, - :subject => 'Issue by anonymous') - - issues = Issue.visible(User.anonymous).all - assert issues.any? - assert_nil issues.detect {|issue| issue.author != User.anonymous} - assert_visibility_match User.anonymous, issues - end - def test_visible_scope_for_anonymous_without_view_issues_permissions # Anonymous user should not see issues without permission Role.anonymous.remove_permission!(:view_issues) @@ -125,6 +113,20 @@ class IssueTest < ActiveSupport::TestCase assert_visibility_match User.anonymous, issues end + def test_anonymous_should_not_see_private_issues_with_issues_visibility_set_to_default + assert Role.anonymous.update_attribute(:issues_visibility, 'default') + issue = Issue.generate_for_project!(Project.find(1), :author => User.anonymous, :assigned_to => User.anonymous, :is_private => true) + assert_nil Issue.where(:id => issue.id).visible(User.anonymous).first + assert !issue.visible?(User.anonymous) + end + + def test_anonymous_should_not_see_private_issues_with_issues_visibility_set_to_own + assert Role.anonymous.update_attribute(:issues_visibility, 'own') + issue = Issue.generate_for_project!(Project.find(1), :author => User.anonymous, :assigned_to => User.anonymous, :is_private => true) + assert_nil Issue.where(:id => issue.id).visible(User.anonymous).first + assert !issue.visible?(User.anonymous) + end + def test_visible_scope_for_non_member user = User.find(9) assert user.projects.empty?