Handle deleted wiki page versions (#10852).

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

diff --git a/app/models/wiki_content.rb b/app/models/wiki_content.rb
index ac7459826921153b73a960ecf09993458f6acefa..ec1cd8234fa1f200883e4e8cc0fa3d85a57e030b 100644 (file)
--- a/app/models/wiki_content.rb
+++ b/app/models/wiki_content.rb
@@ -115,10 +115,18 @@ class WikiContent < ActiveRecord::Base
 
     # Returns the previous version or nil
     def previous
-      @previous ||= WikiContent::Version.find(:first,
-                                              :order => 'version DESC',
-                                              :include => :author,
-                                              :conditions => ["wiki_content_id = ? AND version < ?", wiki_content_id, version])
+      @previous ||= WikiContent::Version.
+        reorder('version DESC').
+        includes(:author).
+        where("wiki_content_id = ? AND version < ?", wiki_content_id, version).first
+    end
+
+    # Returns the next version or nil
+    def next
+      @next ||= WikiContent::Version.
+        reorder('version ASC').
+        includes(:author).
+        where("wiki_content_id = ? AND version > ?", wiki_content_id, version).first
     end
   end
 end

diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb
index 2f7803f2aa99b08f19117b6538d496da39f8afa8..37d6cf5a4061deaee570127223d7769a7aa11c7e 100644 (file)
--- a/app/models/wiki_page.rb
+++ b/app/models/wiki_page.rb
@@ -111,11 +111,12 @@ class WikiPage < ActiveRecord::Base
 
   def diff(version_to=nil, version_from=nil)
     version_to = version_to ? version_to.to_i : self.content.version
-    version_from = version_from ? version_from.to_i : version_to - 1
-    version_to, version_from = version_from, version_to unless version_from < version_to
-
     content_to = content.versions.find_by_version(version_to)
-    content_from = content.versions.find_by_version(version_from)
+    content_from = version_from ? content.versions.find_by_version(version_from.to_i) : content_to.previous
+
+    if content_from.version > content_to.version
+      content_to, content_from = content_from, content_to
+    end
 
     (content_to && content_from) ? WikiDiff.new(content_to, content_from) : nil
   end

diff --git a/app/views/wiki/show.html.erb b/app/views/wiki/show.html.erb
index 7227870ec28e9e9a7397f33a037424acb5bcee85..702b300231cc5aef71a9db800fbce82cf6f7ca31 100644 (file)
--- a/app/views/wiki/show.html.erb
+++ b/app/views/wiki/show.html.erb
@@ -17,14 +17,14 @@
     <p>
     <%= link_to(("\xc2\xab " + l(:label_previous)),
                 :action => 'show', :id => @page.title, :project_id => @page.project,
-                :version => (@content.version - 1)) + " - " if @content.version > 1 %>
+                :version => @content.previous.version) + " - " if @content.previous %>
     <%= "#{l(:label_version)} #{@content.version}/#{@page.content.version}" %>
     <%= '('.html_safe + link_to(l(:label_diff), :controller => 'wiki', :action => 'diff',
                       :id => @page.title, :project_id => @page.project,
-                      :version => @content.version) + ')'.html_safe if @content.version > 1 %> - 
+                      :version => @content.version) + ')'.html_safe if @content.previous %> - 
     <%= link_to((l(:label_next) + " \xc2\xbb"), :action => 'show',
                 :id => @page.title, :project_id => @page.project,
-                :version => (@content.version + 1)) + " - " if @content.version < @page.content.version %>
+                :version => @content.next.version) + " - " if @content.next %>
     <%= link_to(l(:label_current_version), :action => 'show', :id => @page.title, :project_id => @page.project) %>
     <br />
     <em><%= @content.author ? link_to_user(@content.author) : l(:label_user_anonymous)

diff --git a/test/unit/wiki_content_test.rb b/test/unit/wiki_content_test.rb
index 4200958078e51f3b8c55498e8e00d8576083c2c6..b49aba0085c0a1d4c73da6b96790dd2809a1b5fe 100644 (file)
--- a/test/unit/wiki_content_test.rb
+++ b/test/unit/wiki_content_test.rb
@@ -122,4 +122,42 @@ class WikiContentTest < ActiveSupport::TestCase
     assert_equal true, content.versions.first(:order => 'version DESC').current_version?
     assert_equal false, content.versions.first(:order => 'version ASC').current_version?
   end
+
+  def test_previous_for_first_version_should_return_nil
+    content = WikiContent::Version.find_by_page_id_and_version(1, 1)
+    assert_nil content.previous
+  end
+
+  def test_previous_for_version_should_return_previous_version
+    content = WikiContent::Version.find_by_page_id_and_version(1, 3)
+    assert_not_nil content.previous
+    assert_equal 2, content.previous.version
+  end
+
+  def test_previous_for_version_with_gap_should_return_previous_available_version
+    WikiContent::Version.find_by_page_id_and_version(1, 2).destroy
+
+    content = WikiContent::Version.find_by_page_id_and_version(1, 3)
+    assert_not_nil content.previous
+    assert_equal 1, content.previous.version
+  end
+
+  def test_next_for_last_version_should_return_nil
+    content = WikiContent::Version.find_by_page_id_and_version(1, 3)
+    assert_nil content.next
+  end
+
+  def test_next_for_version_should_return_next_version
+    content = WikiContent::Version.find_by_page_id_and_version(1, 1)
+    assert_not_nil content.next
+    assert_equal 2, content.next.version
+  end
+
+  def test_next_for_version_with_gap_should_return_next_available_version
+    WikiContent::Version.find_by_page_id_and_version(1, 2).destroy
+
+    content = WikiContent::Version.find_by_page_id_and_version(1, 1)
+    assert_not_nil content.next
+    assert_equal 3, content.next.version
+  end
 end

diff --git a/test/unit/wiki_page_test.rb b/test/unit/wiki_page_test.rb
index 2a606a7821215dd33d5b060701273b8da122fccc..8eb2af0ea80ddc6fed3e61136b184d92351facdc 100644 (file)
--- a/test/unit/wiki_page_test.rb
+++ b/test/unit/wiki_page_test.rb
@@ -149,4 +149,14 @@ class WikiPageTest < ActiveSupport::TestCase
     assert_equal %w(Child1 Child11 Child2 Parent), page.self_and_descendants(2).map(&:title).sort
     assert_equal %w(Child1 Child2 Parent), page.self_and_descendants(1).map(&:title).sort
   end
+
+  def test_diff_for_page_with_deleted_version_should_pick_the_previous_available_version
+    WikiContent::Version.find_by_page_id_and_version(1, 2).destroy
+
+    page = WikiPage.find(1)
+    diff = page.diff(3)
+    assert_not_nil diff
+    assert_equal 3, diff.content_to.version
+    assert_equal 1, diff.content_from.version
+  end
 end