From 7d501eaf81f32c6cf6e361192968aacbe18ff36c Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Tue, 13 Dec 2011 20:40:03 +0000
Subject: [PATCH] Code cleanup.

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@8200 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/views/issues/_attributes.html.erb         |  2 +-
 app/views/issues/_form.html.erb               |  2 +-
 app/views/issues/new.html.erb                 |  2 +-
 lib/redmine/safe_attributes.rb                | 14 +++++++++++---
 test/unit/lib/redmine/safe_attributes_test.rb | 18 ++++++++++++++++--
 5 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/app/views/issues/_attributes.html.erb b/app/views/issues/_attributes.html.erb
index c4a087dae..6303ccf79 100644
--- a/app/views/issues/_attributes.html.erb
+++ b/app/views/issues/_attributes.html.erb
@@ -31,7 +31,7 @@
 </div>
 
 <div class="splitcontentright">
-<% if User.current.allowed_to?(:manage_subtasks, @project) %>
+<% if @issue.safe_attribute? 'parent_issue_id' %>
 <p id="parent_issue"><%= f.text_field :parent_issue_id, :size => 10 %></p>
 <div id="parent_issue_candidates" class="autocomplete"></div>
 <%= javascript_tag "observeParentIssueField('#{auto_complete_issues_path(:id => @issue, :project_id => @project) }')" %>
diff --git a/app/views/issues/_form.html.erb b/app/views/issues/_form.html.erb
index d8dd91ee6..4b4cbccb0 100644
--- a/app/views/issues/_form.html.erb
+++ b/app/views/issues/_form.html.erb
@@ -1,6 +1,6 @@
 <%= call_hook(:view_issues_form_details_top, { :issue => @issue, :form => f }) %>
 
-<% if @issue.safe_attribute_names.include?('is_private') %>
+<% if @issue.safe_attribute? 'is_private' %>
 <p style="float:right; margin-right:1em;">
   <label class="inline" for="issue_is_private" id="issue_is_private_label"><%= f.check_box :is_private, :no_label => true %> <%= l(:field_is_private) %></label>
 </p>
diff --git a/app/views/issues/new.html.erb b/app/views/issues/new.html.erb
index 48d786879..615263b23 100644
--- a/app/views/issues/new.html.erb
+++ b/app/views/issues/new.html.erb
@@ -10,7 +10,7 @@
 
     <p id="attachments_form"><%= label_tag('attachments[1][file]', l(:label_attachment_plural))%><%= render :partial => 'attachments/form' %></p>
 
-    <% if User.current.allowed_to?(:add_issue_watchers, @project) -%>
+    <% if @issue.safe_attribute? 'watcher_user_ids' -%>
       <p id="watchers_form"><label><%= l(:label_issue_watchers) %></label>
       <% @issue.project.users.sort.each do |user| -%>
         <label class="floating"><%= check_box_tag 'issue[watcher_user_ids][]', user.id, @issue.watched_by?(user) %> <%=h user %></label>
diff --git a/lib/redmine/safe_attributes.rb b/lib/redmine/safe_attributes.rb
index 3c17f952d..3724b437d 100644
--- a/lib/redmine/safe_attributes.rb
+++ b/lib/redmine/safe_attributes.rb
@@ -44,14 +44,22 @@ module Redmine
     # Example:
     #   book.safe_attributes # => ['title', 'pages']
     #   book.safe_attributes(book.author) # => ['title', 'pages', 'isbn']
-    def safe_attribute_names(user=User.current)
+    def safe_attribute_names(user=nil)
+      return @safe_attribute_names if @safe_attribute_names && user.nil?
       names = []
       self.class.safe_attributes.collect do |attrs, options|
-        if options[:if].nil? || options[:if].call(self, user)
+        if options[:if].nil? || options[:if].call(self, user || User.current)
           names += attrs.collect(&:to_s)
         end
       end
-      names.uniq
+      names.uniq!
+      @safe_attribute_names = names if user.nil?
+      names
+    end
+
+    # Returns true if attr can be set by user or the current user
+    def safe_attribute?(attr, user=nil)
+      safe_attribute_names(user).include?(attr.to_s)
     end
 
     # Returns a hash with unsafe attributes removed
diff --git a/test/unit/lib/redmine/safe_attributes_test.rb b/test/unit/lib/redmine/safe_attributes_test.rb
index a8a468027..6a21efc59 100644
--- a/test/unit/lib/redmine/safe_attributes_test.rb
+++ b/test/unit/lib/redmine/safe_attributes_test.rb
@@ -42,16 +42,30 @@ class Redmine::SafeAttributesTest < ActiveSupport::TestCase
 
   def test_safe_attribute_names
     p = Person.new
-    assert_equal ['firstname', 'lastname'], p.safe_attribute_names(User.anonymous)
-    assert_equal ['firstname', 'lastname', 'login'], p.safe_attribute_names(User.find(1))
+    user = User.anonymous
+    assert_equal ['firstname', 'lastname'], p.safe_attribute_names(user)
+    assert p.safe_attribute?('firstname', user)
+    assert !p.safe_attribute?('login', user)
+
+    p = Person.new
+    user = User.find(1)
+    assert_equal ['firstname', 'lastname', 'login'], p.safe_attribute_names(user)
+    assert p.safe_attribute?('firstname', user)
+    assert p.safe_attribute?('login', user)
   end
 
   def test_safe_attribute_names_without_user
     p = Person.new
     User.current = nil
     assert_equal ['firstname', 'lastname'], p.safe_attribute_names
+    assert p.safe_attribute?('firstname')
+    assert !p.safe_attribute?('login')
+
+    p = Person.new
     User.current = User.find(1)
     assert_equal ['firstname', 'lastname', 'login'], p.safe_attribute_names
+    assert p.safe_attribute?('firstname')
+    assert p.safe_attribute?('login')
   end
 
   def test_set_safe_attributes