Wrap journal attributes with a journal parameter and use safe_attributes (#22575).

git-svn-id: http://svn.redmine.org/redmine/trunk@15621 e93f8b46-1217-0410-a6f0-8f06a7374b81

diff --git a/app/controllers/journals_controller.rb b/app/controllers/journals_controller.rb
index 6780916b747256dab99dc17d260330c412247291..00556c8e4442a0a6442f81c74caf34ad80c77e84 100644 (file)
--- a/app/controllers/journals_controller.rb
+++ b/app/controllers/journals_controller.rb
@@ -90,10 +90,8 @@ class JournalsController < ApplicationController
 
   def update
     (render_403; return false) unless @journal.editable_by?(User.current)
-    @journal.notes = params[:notes] if params[:notes]
-    @journal.private_notes = params[:private_notes].present?
-    (render_403; return false) if @journal.private_notes_changed? && User.current.allowed_to?(:set_notes_private, @journal.issue.project) == false
-    @journal.save if @journal.changed?
+    @journal.safe_attributes = params[:journal]
+    @journal.save
     @journal.destroy if @journal.details.empty? && @journal.notes.blank?
     call_hook(:controller_journals_edit_post, { :journal => @journal, :params => params})
     respond_to do |format|

diff --git a/app/helpers/journals_helper.rb b/app/helpers/journals_helper.rb
index e31ad2544b91f5ddc96a2e5ec047aae85c637abd..0cf86364850a97a38c616a7bd0359f6a3c18b2f3 100644 (file)
--- a/app/helpers/journals_helper.rb
+++ b/app/helpers/journals_helper.rb
@@ -45,7 +45,7 @@ module JournalsHelper
                        :class => 'icon-only icon-edit'
                       ) if editable
       links << link_to(l(:button_delete),
-                       journal_path(journal, :notes => ""),
+                       journal_path(journal, :journal => {:notes => ""}),
                        :remote => true,
                        :method => 'put', :data => {:confirm => l(:text_are_you_sure)}, 
                        :title => l(:button_delete),

diff --git a/app/models/journal.rb b/app/models/journal.rb
index 927f86f659da08fc8ba233623db4b38717635639..d65034c567afbe9e366efe3bd81068202b07f2fc 100644 (file)
--- a/app/models/journal.rb
+++ b/app/models/journal.rb
@@ -16,6 +16,8 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 class Journal < ActiveRecord::Base
+  include Redmine::SafeAttributes
+
   belongs_to :journalized, :polymorphic => true
   # added as a quick fix to allow eager loading of the polymorphic association
   # since always associated to an issue, for now
@@ -50,6 +52,11 @@ class Journal < ActiveRecord::Base
       where("(#{Journal.table_name}.private_notes = ? OR (#{Project.allowed_to_condition(user, :view_private_notes, *args)}))", false)
   }
 
+  safe_attributes 'notes',
+    :if => lambda {|journal, user| journal.new_record? || journal.editable_by?(user)}
+  safe_attributes 'private_notes',
+    :if => lambda {|journal, user| user.allowed_to?(:set_notes_private, journal.project)}
+
   def initialize(*args)
     super
     if journalized

diff --git a/app/views/journals/_notes_form.html.erb b/app/views/journals/_notes_form.html.erb
index e8e73bb18eefda38ecc5301afab8722eac98bf2d..3bd6a31fd1a88dc929268b40f31707c008924322 100644 (file)
--- a/app/views/journals/_notes_form.html.erb
+++ b/app/views/journals/_notes_form.html.erb
@@ -3,12 +3,14 @@
              :method => 'put',
              :id => "journal-#{@journal.id}-form") do %>
     <%= label_tag "notes", l(:description_notes), :class => "hidden-for-sighted" %>
-    <%= text_area_tag :notes, @journal.notes,
+    <%= text_area_tag 'journal[notes]', @journal.notes,
           :id => "journal_#{@journal.id}_notes",
           :class => 'wiki-edit',
           :rows => (@journal.notes.blank? ? 10 : [[10, @journal.notes.length / 50].max, 100].min) %>
-    <% if @journal.issue.safe_attribute? 'private_notes' %>
-      <%= check_box_tag 'private_notes', '1', @journal.private_notes, :id => "journal_#{@journal.id}_private_notes" %> <label for="journal_<%= @journal.id %>_private_notes"><%= l(:field_private_notes) %></label>
+    <% if @journal.safe_attribute? 'private_notes' %>
+      <%= hidden_field_tag 'journal[private_notes]', '0' %>
+      <%= check_box_tag 'journal[private_notes]', '1', @journal.private_notes, :id => "journal_#{@journal.id}_private_notes" %>
+      <label for="journal_<%= @journal.id %>_private_notes"><%= l(:field_private_notes) %></label>
     <% end %>
     <%= call_hook(:view_journals_notes_form_after_notes, { :journal => @journal}) %>
     <p><%= submit_tag l(:button_save) %>

diff --git a/test/functional/journals_controller_test.rb b/test/functional/journals_controller_test.rb
index 304f205f783e8f2cb8ef825317a1c867767d7611..424a4aa8bdae836fa0dd720a647f6799c2cc86f4 100644 (file)
--- a/test/functional/journals_controller_test.rb
+++ b/test/functional/journals_controller_test.rb
@@ -199,7 +199,7 @@ class JournalsControllerTest < ActionController::TestCase
 
   def test_update_xhr
     @request.session[:user_id] = 1
-    xhr :post, :update, :id => 2, :notes => 'Updated notes'
+    xhr :post, :update, :id => 2, :journal => {:notes => 'Updated notes'}
     assert_response :success
     assert_template 'update'
     assert_equal 'text/javascript', response.content_type
@@ -209,7 +209,7 @@ class JournalsControllerTest < ActionController::TestCase
 
   def test_update_xhr_with_private_notes_checked
     @request.session[:user_id] = 1
-    xhr :post, :update, :id => 2, :private_notes => '1'
+    xhr :post, :update, :id => 2, :journal => {:private_notes => '1'}
     assert_response :success
     assert_template 'update'
     assert_equal 'text/javascript', response.content_type
@@ -221,7 +221,7 @@ class JournalsControllerTest < ActionController::TestCase
   def test_update_xhr_with_private_notes_unchecked
     Journal.find(2).update_attributes(:private_notes => true)
     @request.session[:user_id] = 1
-    xhr :post, :update, :id => 2
+    xhr :post, :update, :id => 2, :journal => {:private_notes => '0'}
     assert_response :success
     assert_template 'update'
     assert_equal 'text/javascript', response.content_type
@@ -230,20 +230,21 @@ class JournalsControllerTest < ActionController::TestCase
     assert_include 'journal-2-private_notes', response.body
   end
 
-  def test_update_xhr_with_private_notes_changes_and_without_set_private_notes_permission
+  def test_update_xhr_without_set_private_notes_permission_should_ignore_private_notes
     @request.session[:user_id] = 2
     Role.find(1).add_permission! :edit_issue_notes
     Role.find(1).add_permission! :view_private_notes
     Role.find(1).remove_permission! :set_notes_private
 
-    xhr :post, :update, :id => 2, :private_notes => '1'
-    assert_response 403
+    xhr :post, :update, :id => 2, :journal => {:private_notes => '1'}
+    assert_response :success
+    assert_equal false, Journal.find(2).private_notes
   end
 
   def test_update_xhr_with_empty_notes_should_delete_the_journal
     @request.session[:user_id] = 1
     assert_difference 'Journal.count', -1 do
-      xhr :post, :update, :id => 2, :notes => ''
+      xhr :post, :update, :id => 2, :journal => {:notes => ''}
       assert_response :success
       assert_template 'update'
       assert_equal 'text/javascript', response.content_type