Private issues (#7414).

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

app/helpers/issues_helper.rb

@@ -61,18 +61,23 @@ module IssuesHelper
     
   def render_issue_subject_with_tree(issue)
     s = ''
-    ancestors = issue.root? ? [] : issue.ancestors.all
+    ancestors = issue.root? ? [] : issue.ancestors.visible.all
     ancestors.each do |ancestor|
       s << '<div>' + content_tag('p', link_to_issue(ancestor))
     end
-    s << '<div>' + content_tag('h3', h(issue.subject))
+    s << '<div>'
+    subject = h(issue.subject)
+    if issue.is_private?
+      subject = content_tag('span', l(:field_is_private), :class => 'private') + ' ' + subject
+    end
+    s << content_tag('h3', subject)
     s << '</div>' * (ancestors.size + 1)
     s
   end
   
   def render_descendants_tree(issue)
     s = '<form><table class="list issues">'
-    issue_list(issue.descendants.sort_by(&:lft)) do |child, level|
+    issue_list(issue.descendants.visible.sort_by(&:lft)) do |child, level|
       s << content_tag('tr',
              content_tag('td', check_box_tag("ids[]", child.id, false, :id => nil), :class => 'checkbox') +
              content_tag('td', link_to_issue(child, :truncate => 60), :class => 'subject') +
@@ -159,6 +164,10 @@ module IssuesHelper
         label = l(:field_parent_issue)
         value = "##{detail.value}" unless detail.value.blank?
         old_value = "##{detail.old_value}" unless detail.old_value.blank?
+        
+      when detail.prop_key == 'is_private'
+        value = l(detail.value == "0" ? :general_text_No : :general_text_Yes) unless detail.value.blank?
+        old_value = l(detail.old_value == "0" ? :general_text_No : :general_text_Yes) unless detail.old_value.blank?
       end
     when 'cf'
       custom_field = CustomField.find_by_id(detail.prop_key)

app/models/issue.rb

@@ -90,8 +90,10 @@ class Issue < ActiveRecord::Base
   def self.visible_condition(user, options={})
     Project.allowed_to_condition(user, :view_issues, options) do |role, user|
       case role.issues_visibility
-      when 'default'
+      when 'all'
         nil
+      when 'default'
+        "(#{table_name}.is_private = #{connection.quoted_false} OR #{table_name}.author_id = #{user.id} OR #{table_name}.assigned_to_id = #{user.id})"
       when 'own'
         "(#{table_name}.author_id = #{user.id} OR #{table_name}.assigned_to_id = #{user.id})"
       else
@@ -104,8 +106,10 @@ class Issue < ActiveRecord::Base
   def visible?(usr=nil)
     (usr || User.current).allowed_to?(:view_issues, self.project) do |role, user|
       case role.issues_visibility
-      when 'default'
+      when 'all'
         true
+      when 'default'
+        !self.is_private? || self.author == user || self.assigned_to == user
       when 'own'
         self.author == user || self.assigned_to == user
       else
@@ -257,6 +261,12 @@ class Issue < ActiveRecord::Base
     'done_ratio',
     :if => lambda {|issue, user| issue.new_statuses_allowed_to(user).any? }
 
+  safe_attributes 'is_private',
+    :if => lambda {|issue, user|
+      user.allowed_to?(:set_issues_private, issue.project) ||
+        (issue.author == user && user.allowed_to?(:set_own_issues_private, issue.project))
+    }
+  
   # Safely sets attributes
   # Should be called from controllers instead of #attributes=
   # attr_accessible is too rough because we still want things like
@@ -552,6 +562,7 @@ class Issue < ActiveRecord::Base
     s << ' overdue' if overdue?
     s << ' child' if child?
     s << ' parent' unless leaf?
+    s << ' private' if is_private?
     s << ' created-by-me' if User.current.logged? && author_id == User.current.id
     s << ' assigned-to-me' if User.current.logged? && assigned_to_id == User.current.id
     s

app/models/journal_detail.rb

@@ -17,4 +17,22 @@
 
 class JournalDetail < ActiveRecord::Base
   belongs_to :journal
+  before_save :normalize_values
+  
+  private
+  
+  def normalize_values
+    self.value = normalize(value)
+    self.old_value = normalize(old_value)
+  end
+  
+  def normalize(v)
+    if v == true
+      "1"
+    elsif v == false
+      "0"
+    else
+      v
+    end
+  end
 end

app/models/role.rb

@@ -21,7 +21,8 @@ class Role < ActiveRecord::Base
   BUILTIN_ANONYMOUS  = 2
   
   ISSUES_VISIBILITY_OPTIONS = [
-    ['default', :label_issues_visibility_all],
+    ['all', :label_issues_visibility_all],
+    ['default', :label_issues_visibility_public],
     ['own', :label_issues_visibility_own]
   ]
 

app/views/issues/_form.rhtml

@@ -1,6 +1,11 @@
 <%= call_hook(:view_issues_form_details_top, { :issue => @issue, :form => f }) %>
 
 <div id="issue_descr_fields" <%= 'style="display:none"' unless @issue.new_record? || @issue.errors.any? %>>
+<% if @issue.safe_attribute_names.include?('is_private') %>
+<p style="float:right; margin-right:1em;">
+  <label class="inline" for="issue_is_private"><%= f.check_box :is_private, :no_label => true %> <%= l(:field_is_private) %></label>
+</p>
+<% end %>
 <p><%= f.select :tracker_id, @project.trackers.collect {|t| [t.name, t.id]}, :required => true %></p>
 <%= observe_field :issue_tracker_id, :url => { :action => :new, :project_id => @project, :id => @issue },
                                      :update => :attributes,

config/locales/en.yml

@@ -305,6 +305,7 @@ en:
   field_visible: Visible
   field_warn_on_leaving_unsaved: "Warn me when leaving a page with unsaved text"
   field_issues_visibility: Issues visibility
+  field_is_private: Private
   
   setting_app_title: Application title
   setting_app_subtitle: Application subtitle
@@ -377,6 +378,8 @@ en:
   permission_add_issues: Add issues
   permission_edit_issues: Edit issues
   permission_manage_issue_relations: Manage issue relations
+  permission_set_issues_private: Set issues public or private
+  permission_set_own_issues_private: Set own issues public or private
   permission_add_issue_notes: Add notes
   permission_edit_issue_notes: Edit notes
   permission_edit_own_issue_notes: Edit own notes
@@ -806,6 +809,7 @@ en:
   label_additional_workflow_transitions_for_author: Additional transitions allowed when the user is the author
   label_additional_workflow_transitions_for_assignee: Additional transitions allowed when the user is the assignee
   label_issues_visibility_all: All issues
+  label_issues_visibility_public: All non private issues
   label_issues_visibility_own: Issues created by or assigned to the user
   
   button_login: Login

config/locales/fr.yml

@@ -309,6 +309,7 @@ fr:
   field_visible: Visible
   field_warn_on_leaving_unsaved: "M'avertir lorsque je quitte une page contenant du texte non sauvegardé"
   field_issues_visibility: Visibilité des demandes
+  field_is_private: Privée
   
   setting_app_title: Titre de l'application
   setting_app_subtitle: Sous-titre de l'application
@@ -378,6 +379,8 @@ fr:
   permission_add_issues: Créer des demandes
   permission_edit_issues: Modifier les demandes
   permission_manage_issue_relations: Gérer les relations
+  permission_set_issues_private: Rendre les demandes publiques ou privées
+  permission_set_own_issues_private: Rendre ses propres demandes publiques ou privées
   permission_add_issue_notes: Ajouter des notes
   permission_edit_issue_notes: Modifier les notes
   permission_edit_own_issue_notes: Modifier ses propres notes
@@ -793,6 +796,7 @@ fr:
   label_additional_workflow_transitions_for_author: Autorisations supplémentaires lorsque l'utilisateur a créé la demande
   label_additional_workflow_transitions_for_assignee: Autorisations supplémentaires lorsque la demande est assignée à l'utilisateur
   label_issues_visibility_all: Toutes les demandes
+  label_issues_visibility_public: Toutes les demandes non privées
   label_issues_visibility_own: Demandes créées par ou assignées à l'utilisateur
   
   button_login: Connexion

db/migrate/20110412065600_add_issues_is_private.rb

@@ -0,0 +1,9 @@
+class AddIssuesIsPrivate < ActiveRecord::Migration
+  def self.up
+    add_column :issues, :is_private, :boolean, :default => false, :null => false
+  end
+
+  def self.down
+    remove_column :issues, :is_private
+  end
+end

lib/redmine.rb

@@ -71,6 +71,8 @@ Redmine::AccessControl.map do |map|
     map.permission :edit_issues, {:issues => [:edit, :update, :bulk_edit, :bulk_update, :update_form], :journals => [:new]}
     map.permission :manage_issue_relations, {:issue_relations => [:new, :destroy]}
     map.permission :manage_subtasks, {}
+    map.permission :set_issues_private, {}
+    map.permission :set_own_issues_private, {}
     map.permission :add_issue_notes, {:issues => [:edit, :update], :journals => [:new]}
     map.permission :edit_issue_notes, {:journals => :edit}, :require => :loggedin
     map.permission :edit_own_issue_notes, {:journals => :edit}, :require => :loggedin

lib/redmine/default_data/loader.rb

@@ -1,5 +1,5 @@
-# redMine - project management software
-# Copyright (C) 2006-2007  Jean-Philippe Lang
+# Redmine - project management software
+# Copyright (C) 2006-2011  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
@@ -41,6 +41,7 @@ module Redmine
           Role.transaction do
             # Roles
             manager = Role.create! :name => l(:default_role_manager), 
+                                   :issues_visibility => 'all',
                                    :position => 1
             manager.permissions = manager.setable_permissions.collect {|p| p.name}
             manager.save!

public/stylesheets/application.css

@@ -278,6 +278,7 @@ div.issue div.subject div div { padding-left: 16px; }
 div.issue div.subject p {margin: 0; margin-bottom: 0.1em; font-size: 90%; color: #999;}
 div.issue div.subject>div>p { margin-top: 0.5em; }
 div.issue div.subject h3 {margin: 0; margin-bottom: 0.1em;}
+div.issue span.private { position:relative; bottom: 2px; text-transform: uppercase; background: #d22; color: #fff; font-weight:bold; padding: 0px 2px 0px 2px; font-size: 60%; margin-right: 2px; border-radius: 2px; -moz-border-radius: 2px;}
 
 #issue_tree table.issues, #relations table.issues { border: 0; }
 #issue_tree td.checkbox, #relations td.checkbox {display:none;}

test/fixtures/attachments.yml

@@ -169,3 +169,16 @@ attachments_014:
   filename: changeset_utf8.diff
   author_id: 2
   content_type: text/x-diff
+attachments_015: 
+  id: 15
+  created_on: 2010-07-19 21:07:27 +02:00
+  container_type: Issue
+  container_id: 14
+  downloads: 0
+  disk_filename: 060719210727_changeset_utf8.diff
+  digest: b91e08d0cf966d5c6ff411bd8c4cc3a2
+  filesize: 687
+  filename: private.diff
+  author_id: 2
+  content_type: text/x-diff
+  description: attachement of a private issue

test/fixtures/issues.yml

@@ -244,3 +244,21 @@ issues_013:
   root_id: 13
   lft: 1
   rgt: 2
+issues_014:
+  id: 14
+  created_on: <%= 15.days.ago.to_date.to_s(:db) %>
+  project_id: 3
+  updated_on: <%= 15.days.ago.to_date.to_s(:db) %>
+  priority_id: 5
+  subject: Private issue on public project
+  fixed_version_id: 
+  category_id: 
+  description: This is a private issue
+  tracker_id: 1
+  assigned_to_id: 
+  author_id: 2
+  status_id: 1
+  is_private: true
+  root_id: 14
+  lft: 1
+  rgt: 2

test/fixtures/roles.yml

@@ -3,7 +3,7 @@ roles_001:
   name: Manager
   id: 1
   builtin: 0
-  issues_visibility: default
+  issues_visibility: all
   permissions: |
     --- 
     - :add_project

test/functional/attachments_controller_test.rb

@@ -86,6 +86,18 @@ class AttachmentsControllerTest < ActionController::TestCase
     assert_equal 'application/octet-stream', @response.content_type
   end
   
+  def test_show_file_from_private_issue_without_permission
+    get :show, :id => 15
+    assert_redirected_to '/login?back_url=http%3A%2F%2Ftest.host%2Fattachments%2F15'
+  end
+  
+  def test_show_file_from_private_issue_with_permission
+    @request.session[:user_id] = 2
+    get :show, :id => 15
+    assert_response :success
+    assert_tag 'h2', :content => /private.diff/
+  end
+  
   def test_download_text_file
     get :download, :id => 4
     assert_response :success

test/functional/issues_controller_test.rb

@@ -91,6 +91,13 @@ class IssuesControllerTest < ActionController::TestCase
     assert_no_tag :tag => 'a', :content => /Can't print recipes/
     assert_tag :tag => 'a', :content => /Subproject issue/
   end
+
+  def test_index_should_list_visible_issues_only
+    get :index, :per_page => 100
+    assert_response :success
+    assert_not_nil assigns(:issues)
+    assert_nil assigns(:issues).detect {|issue| !issue.visible?}
+  end
   
   def test_index_with_project
     Setting.display_subprojects_issues = 0
@@ -317,6 +324,12 @@ class IssuesControllerTest < ActionController::TestCase
     assert_response :redirect
   end
   
+  def test_show_should_deny_anonymous_access_to_private_issue
+    Issue.update_all(["is_private = ?", true], "id = 1")
+    get :show, :id => 1
+    assert_response :redirect
+  end
+  
   def test_show_should_deny_non_member_access_without_permission
     Role.non_member.remove_permission!(:view_issues)
     @request.session[:user_id] = 9
@@ -324,6 +337,13 @@ class IssuesControllerTest < ActionController::TestCase
     assert_response 403
   end
   
+  def test_show_should_deny_non_member_access_to_private_issue
+    Issue.update_all(["is_private = ?", true], "id = 1")
+    @request.session[:user_id] = 9
+    get :show, :id => 1
+    assert_response 403
+  end
+  
   def test_show_should_deny_member_access_without_permission
     Role.find(1).remove_permission!(:view_issues)
     @request.session[:user_id] = 2
@@ -331,6 +351,35 @@ class IssuesControllerTest < ActionController::TestCase
     assert_response 403
   end
   
+  def test_show_should_deny_member_access_to_private_issue_without_permission
+    Issue.update_all(["is_private = ?", true], "id = 1")
+    @request.session[:user_id] = 3
+    get :show, :id => 1
+    assert_response 403
+  end
+  
+  def test_show_should_allow_author_access_to_private_issue
+    Issue.update_all(["is_private = ?, author_id = 3", true], "id = 1")
+    @request.session[:user_id] = 3
+    get :show, :id => 1
+    assert_response :success
+  end
+  
+  def test_show_should_allow_assignee_access_to_private_issue
+    Issue.update_all(["is_private = ?, assigned_to_id = 3", true], "id = 1")
+    @request.session[:user_id] = 3
+    get :show, :id => 1
+    assert_response :success
+  end
+  
+  def test_show_should_allow_member_access_to_private_issue_with_permission
+    Issue.update_all(["is_private = ?", true], "id = 1")
+    User.find(3).roles_for_project(Project.find(1)).first.update_attribute :issues_visibility, 'all'
+    @request.session[:user_id] = 3
+    get :show, :id => 1
+    assert_response :success
+  end
+  
   def test_show_should_not_disclose_relations_to_invisible_issues
     Setting.cross_project_issue_relations = '1'
     IssueRelation.create!(:issue_from => Issue.find(1), :issue_to => Issue.find(2), :relation_type => 'relates')

test/unit/issue_test.rb

@@ -74,6 +74,7 @@ class IssueTest < ActiveSupport::TestCase
     issues = Issue.visible(User.anonymous).all
     assert issues.any?
     assert_nil issues.detect {|issue| !issue.project.is_public?}
+    assert_nil issues.detect {|issue| issue.is_private?}
     assert_visibility_match User.anonymous, issues
   end
   
@@ -102,6 +103,7 @@ class IssueTest < ActiveSupport::TestCase
     issues = Issue.visible(user).all
     assert issues.any?
     assert_nil issues.detect {|issue| !issue.project.is_public?}
+    assert_nil issues.detect {|issue| issue.is_private?}
     assert_visibility_match user, issues
   end
   
@@ -130,10 +132,11 @@ class IssueTest < ActiveSupport::TestCase
     user = User.find(9)
     # User should see issues of projects for which he has view_issues permissions only
     Role.non_member.remove_permission!(:view_issues)
-    Member.create!(:principal => user, :project_id => 2, :role_ids => [1])
+    Member.create!(:principal => user, :project_id => 3, :role_ids => [2])
     issues = Issue.visible(user).all
     assert issues.any?
-    assert_nil issues.detect {|issue| issue.project_id != 2}
+    assert_nil issues.detect {|issue| issue.project_id != 3}
+    assert_nil issues.detect {|issue| issue.is_private?}
     assert_visibility_match user, issues
   end
   
@@ -145,6 +148,8 @@ class IssueTest < ActiveSupport::TestCase
     assert issues.any?
     # Admin should see issues on private projects that he does not belong to
     assert issues.detect {|issue| !issue.project.is_public?}
+    # Admin should see private issues of other users
+    assert issues.detect {|issue| issue.is_private? && issue.author != user}
     assert_visibility_match user, issues
   end
   

vendor/plugins/acts_as_attachable/lib/acts_as_attachable.rb

@@ -1,5 +1,5 @@
 # Redmine - project management software
-# Copyright (C) 2006-2008  Jean-Philippe Lang
+# Copyright (C) 2006-2011  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
@@ -44,11 +44,13 @@ module Redmine
         end
         
         def attachments_visible?(user=User.current)
-          user.allowed_to?(self.class.attachable_options[:view_permission], self.project)
+          (respond_to?(:visible?) ? visible?(user) : true) &&
+            user.allowed_to?(self.class.attachable_options[:view_permission], self.project)
         end
         
         def attachments_deletable?(user=User.current)
-          user.allowed_to?(self.class.attachable_options[:delete_permission], self.project)
+          (respond_to?(:visible?) ? visible?(user) : true) &&
+            user.allowed_to?(self.class.attachable_options[:delete_permission], self.project)
         end
 
         def initialize_unsaved_attachments