From 04a8f8027300a7f84ab00d878b9d4e682612dce1 Mon Sep 17 00:00:00 2001 From: Alexey Verkhovsky Date: Sun, 29 May 2005 18:40:25 +0000 Subject: [PATCH] Further tweaking of markup error handling code --- app/controllers/admin_controller.rb | 16 +++---- app/controllers/wiki_controller.rb | 30 ++++++------ app/helpers/application_helper.rb | 6 ++- app/models/page.rb | 4 ++ app/models/revision.rb | 10 ++-- app/models/web.rb | 2 +- app/models/wiki_service.rb | 64 ++++++++++++------------- app/views/layouts/default.rhtml | 4 +- app/views/wiki/edit.rhtml | 4 +- app/views/wiki/new.rhtml | 2 +- test/functional/wiki_controller_test.rb | 3 +- 11 files changed, 77 insertions(+), 68 deletions(-) diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 36fda671..6bca0f15 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -6,19 +6,17 @@ class AdminController < ApplicationController def create_system if @wiki.setup? - flash[:error] = <<-EOL - Wiki has already been created in '#{@wiki.storage_path}'. Shut down Instiki and delete - this directory if you want to recreate it from scratch.

- (WARNING: this will destroy content of your current wiki). - EOL + flash[:error] = + "Wiki has already been created in '#{@wiki.storage_path}'. " + + "Shut down Instiki and delete this directory if you want to recreate it from scratch." + + "\n\n" + + "(WARNING: this will destroy content of your current wiki)." redirect_home(@wiki.webs.keys.first) elsif @params['web_name'] # form submitted -> create a wiki @wiki.setup(@params['password'], @params['web_name'], @params['web_address']) - flash[:info] = <<-EOL - Your new wiki '#{@params['web_name']}' is created!
- Please edit its home page and press Submit when finished. - EOL + flash[:info] = "Your new wiki '#{@params['web_name']}' is created!\n" + + "Please edit its home page and press Submit when finished." redirect_to :web => @params['web_address'], :controller => 'wiki', :action => 'new', :id => 'HomePage' else diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb index 8852f24f..43d55b2e 100644 --- a/app/controllers/wiki_controller.rb +++ b/app/controllers/wiki_controller.rb @@ -122,7 +122,7 @@ class WikiController < ApplicationController def edit if @page.nil? - redirect_to :action => 'index' + redirect_home elsif @page.locked?(Time.now) and not @params['break_lock'] redirect_to :web => @web_name, :action => 'locked', :id => @page_name else @@ -172,28 +172,28 @@ class WikiController < ApplicationController end def save - redirect_to :action => 'index' if @page_name.nil? + redirect_home if @page_name.nil? cookies['author'] = @params['author'] begin - page = @web.pages[@page_name] - if @web.pages[@page_name] - wiki.revise_page( - @web_name, @page_name, @params['content'], Time.now, - Author.new(@params['author'], remote_ip) - ) - page.unlock + if @page + wiki.revise_page(@web_name, @page_name, @params['content'], Time.now, + Author.new(@params['author'], remote_ip)) + @page.unlock else - wiki.write_page( - @web_name, @page_name, @params['content'], Time.now, - Author.new(@params['author'], remote_ip) - ) + wiki.write_page(@web_name, @page_name, @params['content'], Time.now, + Author.new(@params['author'], remote_ip)) end redirect_to_page @page_name rescue => e - page.unlock if defined? page flash[:error] = e - redirect_to :action => 'edit', :web => @web_name, :id => @page_name + flash[:content] = @params['content'] + if @page + @page.unlock + redirect_to :action => 'edit', :web => @web_name, :id => @page_name + else + redirect_to :action => 'new', :web => @web_name, :id => @page_name + end end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 4e244464..9be8afc1 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -41,7 +41,6 @@ module ApplicationHelper html_options) end - # Creates a hyperlink to a Wiki page, or to a "new page" form if the page doesn't exist yet def link_to_page(page_name, web = @web, text = nil, options = {}) raise 'Web not defined' if web.nil? @@ -65,4 +64,9 @@ module ApplicationHelper end end + # Performs HTML escaping on text, but keeps linefeeds intact (by replacing them with
) + def escape_preserving_linefeeds(text) + h(text).gsub(/\n/, '
') + end + end diff --git a/app/models/page.rb b/app/models/page.rb index 9c5bba8a..5926fb85 100644 --- a/app/models/page.rb +++ b/app/models/page.rb @@ -23,6 +23,10 @@ class Page "You have tried to save page '#{name}' without changing its content") end + # Try to render content to make sure that markup engine can take it, + # before addin a revision to the page + Revision.new(self, @revisions.length, content, created_at, author).force_rendering + # A user may change a page, look at it and make some more changes - several times. # Not to record every such iteration as a new revision, if the previous revision was done # by the same author, not more than 30 minutes ago, then update the last revision instead of diff --git a/app/models/revision.rb b/app/models/revision.rb index 248ae12a..c5f0eb8a 100644 --- a/app/models/revision.rb +++ b/app/models/revision.rb @@ -26,6 +26,8 @@ class Revision ).strftime "%B %e, %Y %H:%M" end + +# todo: drop next_revision, previuous_revision and number from here - unused code def next_revision page.revisions[number + 1] end @@ -107,15 +109,17 @@ class Revision def force_rendering begin display_content.render! - rescue Exception => e + rescue => e ApplicationController.logger.error "Failed rendering page #{@name}" ApplicationController.logger.error e - message = e.message.gsub(/\n/, '
') + message = e.message # substitute content with an error message - content = <<-EOL + self.content = <<-EOL

Markup engine has failed to render this page, raising the following error:

#{message}

+ 
#{self.content}
EOL + clear_display_cache raise e end end diff --git a/app/models/web.rb b/app/models/web.rb index 661f8811..17b11434 100644 --- a/app/models/web.rb +++ b/app/models/web.rb @@ -46,8 +46,8 @@ class Web def add_page(name, content, created_at, author) page = Page.new(self, name) - @pages[page.name] = page page.revise(content, created_at, author) + @pages[page.name] = page end def address=(the_address) diff --git a/app/models/wiki_service.rb b/app/models/wiki_service.rb index e2f0fd43..c7f8d515 100644 --- a/app/models/wiki_service.rb +++ b/app/models/wiki_service.rb @@ -38,6 +38,34 @@ module AbstractWikiService @system = {} end + def edit_web(old_address, new_address, name, markup, color, additional_style, safe_mode = false, + password = nil, published = false, brackets_only = false, count_pages = false, + allow_uploads = true, max_upload_size = nil) + + if not @webs.key? old_address + raise Instiki::ValidationError.new("Web with address '#{old_address}' does not exist") + end + + if old_address != new_address + if @webs.key? new_address + raise Instiki::ValidationError.new("There is already a web with address '#{new_address}'") + end + @webs[new_address] = @webs[old_address] + @webs.delete(old_address) + @webs[new_address].address = new_address + end + + web = @webs[new_address] + web.refresh_revisions if settings_changed?(web, markup, safe_mode, brackets_only) + + web.name, web.markup, web.color, web.additional_style, web.safe_mode = + name, markup, color, additional_style, safe_mode + + web.password, web.published, web.brackets_only, web.count_pages = + password, published, brackets_only, count_pages, allow_uploads + web.allow_uploads, web.max_upload_size = allow_uploads, max_upload_size.to_i + end + def read_page(web_address, page_name) ApplicationController.logger.debug "Reading page '#{page_name}' from web '#{web_address}'" web = @webs[web_address] @@ -74,42 +102,14 @@ module AbstractWikiService not (@webs.empty?) end - def edit_web(old_address, new_address, name, markup, color, additional_style, safe_mode = false, - password = nil, published = false, brackets_only = false, count_pages = false, - allow_uploads = true, max_upload_size = nil) - - if not @webs.key? old_address - raise Instiki::ValidationError.new("Web with address '#{old_address}' does not exist") - end - - if old_address != new_address - if @webs.key? new_address - raise Instiki::ValidationError.new("There is already a web with address '#{new_address}'") - end - @webs[new_address] = @webs[old_address] - @webs.delete(old_address) - @webs[new_address].address = new_address - end - - web = @webs[new_address] - web.refresh_revisions if settings_changed?(web, markup, safe_mode, brackets_only) - - web.name, web.markup, web.color, web.additional_style, web.safe_mode = - name, markup, color, additional_style, safe_mode - - web.password, web.published, web.brackets_only, web.count_pages = - password, published, brackets_only, count_pages, allow_uploads - web.allow_uploads, web.max_upload_size = allow_uploads, max_upload_size.to_i - end - - def write_page(web_address, page_name, content, written_on, author) - @webs[web_address].add_page(page_name, content, written_on, author) - end - def storage_path self.class.storage_path end + def write_page(web_address, page_name, content, written_on, author) + @webs[web_address].add_page(page_name, content, written_on, author) + end + private def settings_changed?(web, markup, safe_mode, brackets_only) web.markup != markup || diff --git a/app/views/layouts/default.rhtml b/app/views/layouts/default.rhtml index 83b47cf7..0dc5b2b0 100644 --- a/app/views/layouts/default.rhtml +++ b/app/views/layouts/default.rhtml @@ -61,11 +61,11 @@ PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" <% if @error or @flash[:error] %>
-

<%= h(@error || @flash[:error]) %>

+

<%= escape_preserving_linefeeds(@error || @flash[:error]) %>

<% end %> <% if @flash[:info] %>
-

<%= h @flash[:info] %>

+

<%= escape_preserving_linefeeds @flash[:info] %>

<% end %> <%= render 'navigation' unless @web.nil? || @hide_navigation %> diff --git a/app/views/wiki/edit.rhtml b/app/views/wiki/edit.rhtml index 2e09c88e..cdf3d5d1 100644 --- a/app/views/wiki/edit.rhtml +++ b/app/views/wiki/edit.rhtml @@ -4,8 +4,6 @@ @hide_navigation = true %> -<%= "

Please correct the error that caused this error in rendering:
#{@params["msg"]}

" if @params["msg"] %> -
<%= render("#{@web.markup}_help") %> <%= render 'wiki_words_help' %> @@ -16,7 +14,7 @@ %>

- +

as diff --git a/app/views/wiki/new.rhtml b/app/views/wiki/new.rhtml index 404b793b..0f46b5ad 100644 --- a/app/views/wiki/new.rhtml +++ b/app/views/wiki/new.rhtml @@ -14,7 +14,7 @@ %>

- +

as diff --git a/test/functional/wiki_controller_test.rb b/test/functional/wiki_controller_test.rb index d5420154..bc79a8bf 100755 --- a/test/functional/wiki_controller_test.rb +++ b/test/functional/wiki_controller_test.rb @@ -89,7 +89,8 @@ class WikiControllerTest < Test::Unit::TestCase def test_edit_unknown_page process 'edit', 'web' => 'wiki1', 'id' => 'UnknownPage', 'break_lock' => 'y' - assert_redirected_to :action => 'index' + assert_redirected_to :controller => 'wiki', :action => 'show', :web => 'wiki1', + :id => 'HomePage' end def test_edit_page_with_special_symbols