From cac3b1e5381d0756ba9bb6bcb356a4b081f3407d Mon Sep 17 00:00:00 2001 From: Eric Davis Date: Fri, 22 Oct 2010 16:20:20 +0000 Subject: [PATCH] Refactor: split WikiController#edit into #update update will handle the saving and should be accessed via POST only. git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@4272 e93f8b46-1217-0410-a6f0-8f06a7374b81 --- app/controllers/wiki_controller.rb | 60 +++++++++++++++---------- app/views/wiki/edit.rhtml | 2 +- config/routes.rb | 4 +- lib/redmine.rb | 2 +- test/functional/wiki_controller_test.rb | 4 +- test/integration/routing_test.rb | 2 +- 6 files changed, 45 insertions(+), 29 deletions(-) diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb index 95e1943e9cc..1c42ab5cd97 100644 --- a/app/controllers/wiki_controller.rb +++ b/app/controllers/wiki_controller.rb @@ -71,34 +71,48 @@ def edit @content.text = initial_page_content(@page) if @content.text.blank? # don't keep previous comment @content.comments = nil - if request.get? - # To prevent StaleObjectError exception when reverting to a previous version - @content.version = @page.content.version - else - if !@page.new_record? && @content.text == params[:content][:text] - attachments = Attachment.attach_files(@page, params[:attachments]) - render_attachment_warning_if_needed(@page) - # don't save if text wasn't changed - redirect_to :action => 'show', :project_id => @project, :page => @page.title - return - end - #@content.text = params[:content][:text] - #@content.comments = params[:content][:comments] - @content.attributes = params[:content] - @content.author = User.current - # if page is new @page.save will also save content, but not if page isn't a new record - if (@page.new_record? ? @page.save : @content.save) - attachments = Attachment.attach_files(@page, params[:attachments]) - render_attachment_warning_if_needed(@page) - call_hook(:controller_wiki_edit_after_save, { :params => params, :page => @page}) - redirect_to :action => 'show', :project_id => @project, :page => @page.title - end + + # To prevent StaleObjectError exception when reverting to a previous version + @content.version = @page.content.version + rescue ActiveRecord::StaleObjectError + # Optimistic locking exception + flash[:error] = l(:notice_locking_conflict) + end + + verify :method => :post, :only => :update, :render => {:nothing => true, :status => :method_not_allowed } + # Creates a new page or updates an existing one + def update + @page = @wiki.find_or_new_page(params[:page]) + return render_403 unless editable? + @page.content = WikiContent.new(:page => @page) if @page.new_record? + + @content = @page.content_for_version(params[:version]) + @content.text = initial_page_content(@page) if @content.text.blank? + # don't keep previous comment + @content.comments = nil + + if !@page.new_record? && params[:content].present? && @content.text == params[:content][:text] + attachments = Attachment.attach_files(@page, params[:attachments]) + render_attachment_warning_if_needed(@page) + # don't save if text wasn't changed + redirect_to :action => 'show', :project_id => @project, :page => @page.title + return + end + @content.attributes = params[:content] + @content.author = User.current + # if page is new @page.save will also save content, but not if page isn't a new record + if (@page.new_record? ? @page.save : @content.save) + attachments = Attachment.attach_files(@page, params[:attachments]) + render_attachment_warning_if_needed(@page) + call_hook(:controller_wiki_edit_after_save, { :params => params, :page => @page}) + redirect_to :action => 'show', :project_id => @project, :page => @page.title end + rescue ActiveRecord::StaleObjectError # Optimistic locking exception flash[:error] = l(:notice_locking_conflict) end - + # rename a page def rename return render_403 unless editable? diff --git a/app/views/wiki/edit.rhtml b/app/views/wiki/edit.rhtml index 19052a318ac..b14bfb720b3 100644 --- a/app/views/wiki/edit.rhtml +++ b/app/views/wiki/edit.rhtml @@ -1,6 +1,6 @@

<%=h @page.pretty_title %>

-<% form_for :content, @content, :url => {:action => 'edit', :page => @page.title}, :html => {:multipart => true, :id => 'wiki_form'} do |f| %> +<% form_for :content, @content, :url => {:action => 'update', :page => @page.title}, :html => {:multipart => true, :id => 'wiki_form'} do |f| %> <%= f.hidden_field :version %> <%= error_messages_for 'content' %> diff --git a/config/routes.rb b/config/routes.rb index 84d00c687a5..5676f4546ba 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -41,8 +41,10 @@ end wiki_routes.connect 'projects/:project_id/wiki/:page/:action', - :action => /edit|rename|destroy|preview|protect/, + :action => /rename|destroy|preview|protect/, :conditions => {:method => :post} + + wiki_routes.connect 'projects/:project_id/wiki/:page/edit', :action => 'update', :conditions => {:method => :post} end map.with_options :controller => 'messages' do |messages_routes| diff --git a/lib/redmine.rb b/lib/redmine.rb index d5825e00fa2..1a4b581ceaf 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -114,7 +114,7 @@ map.permission :view_wiki_pages, :wiki => [:show, :special, :page_index, :date_index] map.permission :export_wiki_pages, :wiki => [:export] map.permission :view_wiki_edits, :wiki => [:history, :diff, :annotate] - map.permission :edit_wiki_pages, :wiki => [:edit, :preview, :add_attachment] + map.permission :edit_wiki_pages, :wiki => [:edit, :update, :preview, :add_attachment] map.permission :delete_wiki_pages_attachments, {} map.permission :protect_wiki_pages, {:wiki => :protect}, :require => :member end diff --git a/test/functional/wiki_controller_test.rb b/test/functional/wiki_controller_test.rb index cd8c3448d6b..61e265925a0 100644 --- a/test/functional/wiki_controller_test.rb +++ b/test/functional/wiki_controller_test.rb @@ -80,7 +80,7 @@ def test_show_unexistent_page_with_edit_right def test_create_page @request.session[:user_id] = 2 - post :edit, :project_id => 1, + post :update, :project_id => 1, :page => 'New page', :content => {:comments => 'Created the page', :text => "h1. New page\n\nThis is a new page", @@ -96,7 +96,7 @@ def test_create_page_with_attachments @request.session[:user_id] = 2 assert_difference 'WikiPage.count' do assert_difference 'Attachment.count' do - post :edit, :project_id => 1, + post :update, :project_id => 1, :page => 'New page', :content => {:comments => 'Created the page', :text => "h1. New page\n\nThis is a new page", diff --git a/test/integration/routing_test.rb b/test/integration/routing_test.rb index f44d3c94e81..084330c1c1d 100644 --- a/test/integration/routing_test.rb +++ b/test/integration/routing_test.rb @@ -322,7 +322,7 @@ class RoutingTest < ActionController::IntegrationTest should_route :get, "/projects/567/wiki/date_index", :controller => 'wiki', :action => 'date_index', :project_id => '567' should_route :get, "/projects/567/wiki/export", :controller => 'wiki', :action => 'export', :project_id => '567' - should_route :post, "/projects/567/wiki/my_page/edit", :controller => 'wiki', :action => 'edit', :project_id => '567', :page => 'my_page' + should_route :post, "/projects/567/wiki/my_page/edit", :controller => 'wiki', :action => 'update', :project_id => '567', :page => 'my_page' should_route :post, "/projects/567/wiki/CookBook_documentation/preview", :controller => 'wiki', :action => 'preview', :project_id => '567', :page => 'CookBook_documentation' should_route :post, "/projects/22/wiki/ladida/rename", :controller => 'wiki', :action => 'rename', :project_id => '22', :page => 'ladida' should_route :post, "/projects/22/wiki/ladida/destroy", :controller => 'wiki', :action => 'destroy', :project_id => '22', :page => 'ladida'