Dropped number from revision table. Also dropped timestamp. We will rely on autoincremented ID for sorting, and will for now store the time of last edit of the revision in revised_at. Later we will refactor content into a separate table (so as not to load the whole 300 kb of text and cached HTML every time we need page.revisions in code). Rake tests all pass, but watir tests indicate that some revision traversing links are still broken

This commit is contained in:
Alexey Verkhovsky 2005-08-14 22:26:54 +00:00
parent 476d7810f6
commit 052754b068
18 changed files with 116 additions and 112 deletions

View file

@ -140,7 +140,7 @@ class WikiController < ApplicationController
def pdf
page = wiki.read_page(@web_name, @page_name)
safe_page_name = @page.name.gsub(/\W/, '')
file_name = "#{safe_page_name}-#{@web.address}-#{@page.created_at.strftime('%Y-%m-%d-%H-%M-%S')}"
file_name = "#{safe_page_name}-#{@web.address}-#{@page.revised_on.strftime('%Y-%m-%d-%H-%M-%S')}"
file_path = File.join(@wiki.storage_path, file_name)
export_page_to_tex("#{file_path}.tex") unless FileTest.exists?("#{file_path}.tex")
@ -274,7 +274,8 @@ class WikiController < ApplicationController
end
def get_page_and_revision
@revision = @page.revisions[@params['rev'].to_i]
@revision_number = @params['rev'].to_i
@revision = @page.revisions[@revision_number]
end
def parse_category
@ -312,8 +313,8 @@ class WikiController < ApplicationController
@pages_by_revision = @web.select.by_revision.first(limit)
else
@pages_by_revision = @web.select.by_revision
@pages_by_revision.reject! { |page| page.created_at < start_date } if start_date
@pages_by_revision.reject! { |page| page.created_at > end_date } if end_date
@pages_by_revision.reject! { |page| page.revised_on < start_date } if start_date
@pages_by_revision.reject! { |page| page.revised_on > end_date } if end_date
end
@hide_description = hide_description

View file

@ -1,9 +1,9 @@
class Page < ActiveRecord::Base
belongs_to :web
has_many :revisions, :order => 'number'
has_one :current_revision, :class_name => 'Revision', :order => 'number DESC'
has_many :revisions, :order => 'id'
has_one :current_revision, :class_name => 'Revision', :order => 'id DESC'
def revise(content, created_at, author)
def revise(content, time, author)
revisions_size = new_record? ? 0 : revisions.size
if (revisions_size > 0) and content == current_revision.content
raise Instiki::ValidationError.new(
@ -13,37 +13,49 @@ class Page < ActiveRecord::Base
author = Author.new(author.to_s) unless author.is_a?(Author)
# Try to render content to make sure that markup engine can take it,
# before addin a revision to the page
Revision.new(:page => self, :content => content, :created_at => created_at, :author => author).force_rendering
Revision.new(:page => self, :content => content, :author => author, :revised_at => time).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
# creating a new one
if (revisions_size > 0) && continous_revision?(created_at, author)
current_revision.update_attributes(:created_at => created_at, :content => content)
if (revisions_size > 1) && continous_revision?(time, author)
current_revision.update_attributes(:content => content, :revised_at => time)
else
Revision.create(:page => self, :content => content, :created_at => created_at, :author => author)
Revision.create(:page => self, :content => content, :author => author, :revised_at => time)
end
self.created_at = created_at
save
web.refresh_pages_with_references(name) if revisions_size == 0
self
end
def rollback(revision_number, created_at, author_ip = nil)
roll_back_revision = Revision.find(:first, :conditions => ['page_id = ? AND number = ?', id, revision_number])
revise(roll_back_revision.content, created_at, Author.new(roll_back_revision.author, author_ip))
def rollback(revision_number, time, author_ip = nil)
roll_back_revision = self.revisions[revision_number]
if roll_back_revision.nil?
raise Instiki::ValidationError.new("Revision #{revision_number} not found")
end
revise(roll_back_revision.content, time, Author.new(roll_back_revision.author, author_ip))
end
def revisions?
revisions.size > 1
end
def revised_on
created_on
def previous_revision(revision)
revision_index = revisions.each_with_index do |rev, index|
if rev.id == revision.id
break index
else
nil
end
end
if revision_index.nil? or revision_index == 0
nil
else
revisions[revision_index - 1]
end
end
def in_category?(cat)
@ -108,8 +120,8 @@ class Page < ActiveRecord::Base
private
def continous_revision?(created_at, author)
current_revision.author == author && current_revision.created_at + 30.minutes > created_at
def continous_revision?(time, author)
(current_revision.author == author) && (revised_on + 30.minutes > time)
end
# Forward method calls to the current revision, so the page responds to all revision calls

View file

@ -17,10 +17,9 @@ class PageSet < Array
end
def most_recent_revision
self.map { |page| page.created_at }.max || Time.at(0)
self.map { |page| page.revised_on }.max || Time.at(0)
end
def by_name
PageSet.new(@web, sort_by { |page| page.name })
end
@ -28,7 +27,7 @@ class PageSet < Array
alias :sort :by_name
def by_revision
PageSet.new(@web, sort_by { |page| page.created_at }).reverse
PageSet.new(@web, sort_by { |page| page.revised_on }).reverse
end
def pages_that_reference(page_name)

View file

@ -3,24 +3,13 @@ class Revision < ActiveRecord::Base
belongs_to :page
composed_of :author, :mapping => [ %w(author name), %w(ip ip) ]
def created_on
created_at.to_date
def revised_on
revised_at
end
def pretty_created_at
# Must use DateTime because Time doesn't support %e on at least some platforms
DateTime.new(
created_at.year, created_at.mon, created_at.day, created_at.hour, created_at.min
).strftime "%B %e, %Y %H:%M"
end
# todo: drop next_revision, previuous_revision and number from here - unused code
def next_revision
Revision.find_by_number_and_page_id(number+1, page_id)
end
def previous_revision
@previous_revions ||= number > 0 ? Revision.find_by_number_and_page_id(number-1, page_id) : nil
# TODO this method belongs in the view helpers (only views use it)
def pretty_created_on
revised_on.to_date.strftime "%B %e, %Y %H:%M:%S"
end
# Returns an array of all the WikiIncludes present in the content of this revision.
@ -30,7 +19,7 @@ class Revision < ActiveRecord::Base
@wiki_includes_cache = chunks.map { |c| ( c.escaped? ? nil : c.page_name ) }.compact.uniq
end
@wiki_includes_cache
end
end
# Returns an array of all the WikiReferences present in the content of this revision.
def wiki_references
@ -72,8 +61,14 @@ class Revision < ActiveRecord::Base
@display_cache
end
# TODO this probably doesn't belong in revision (because it has to call back the page)
def display_diff
previous_revision ? HTMLDiff.diff(previous_revision.display_content, display_content) : display_content
previous_revision = page.previous_revision(self)
if previous_revision
HTMLDiff.diff(previous_revision.display_content, display_content)
else
display_content
end
end
def clear_display_cache
@ -112,18 +107,8 @@ class Revision < ActiveRecord::Base
end
protected
before_create :set_revision_number, :set_timestamp
after_create :force_rendering
after_save :clear_display_cache
# TODO Refactor this away. Revisions collection should not rely on the revision number for
# sorting etc - revisions must be easy to delete (this helps fighting wiki spam)
def set_revision_number
self.number = self.class.count(['page_id = ?', page_id]) + 1
end
def set_timestamp
self.timestamp = (Time.now.to_f * 1000).to_i.to_s
end
end

View file

@ -17,9 +17,9 @@ class Web < ActiveRecord::Base
self.brackets_only != brackets_only
end
def add_page(name, content, created_at, author)
def add_page(name, content, time, author)
page = page(name) || Page.new(:web => self, :name => name)
page.revise(content, created_at, author)
page.revise(content, time, author)
end
def authors

View file

@ -64,9 +64,9 @@ class Wiki
page.revise(content, revised_on, author)
end
def rollback_page(web_address, page_name, revision_number, created_at, author_id = nil)
def rollback_page(web_address, page_name, revision_number, time, author_id = nil)
page = read_page(web_address, page_name)
page.rollback(revision_number, created_at, author_id)
page.rollback(revision_number, time, author_id)
end
def setup(password, web_name, web_address)

View file

@ -12,7 +12,7 @@
<div id="changes" style="display: none">
<p style="background: #eee; padding: 3px; border: 1px solid silver">
<small>
Showing changes from revision #<%= @page.number - 1 %> to #<%= @page.number %>:
Showing changes from revision #<%= @page.revisions.size - 1 %> to #<%= @page.revisions.size %>:
<ins class="diffins">Added</ins> | <del class="diffdel">Removed</del>
</small>
</p>
@ -22,7 +22,7 @@
<% end %>
<div class="byline">
<%= @page.revisions? ? "Revised" : "Created" %> on <%= @page.pretty_created_at %>
<%= @page.revisions? ? "Revised" : "Created" %> on <%= @page.pretty_created_on %>
by <%= @page.author_link %>
<%= "(#{@page.author.ip})" if @page.author.respond_to?(:ip) %>
<% if @web.count_pages? %>

View file

@ -8,7 +8,7 @@
<%= @page.display_content_for_export %>
<div class="byline">
<%= @page.revisions? ? "Revised" : "Created" %> on <%= @page.pretty_created_at %>
<%= @page.revisions? ? "Revised" : "Created" %> on <%= @page.pretty_created_on %>
by
<%= @page.author_link({ :mode => (@link_mode || :show) }) %>
</div>

View file

@ -17,7 +17,7 @@
<%= link_to_existing_page page %>
<div class="byline" style="margin-bottom: 0px">
by <%= link_to_page(page.author) %>
at <%= page.created_at.strftime "%H:%M" %>
at <%= page.current_revision.pretty_created_on %>
<%= "from #{page.author.ip}" if page.author.respond_to?(:ip) %>
</div>
</li>

View file

@ -1,4 +1,4 @@
<% @title = "#{@page.plain_name} (Rev ##{@revision.number})" %>
<% @title = "#{@page.plain_name} (Rev ##{@revision_number})" %>
<div id="revision">
<%= @revision.display_content %>
@ -7,7 +7,7 @@
<div id="changes" style="display: none">
<p style="background: #eee; padding: 3px; border: 1px solid silver">
<small>
Showing changes from revision #<%= @revision.number - 1 %> to #<%= @revision.number %>:
Showing changes from revision #<%= @revision_number - 1 %> to #<%= @revision_number %>:
<ins class="diffins">Added</ins> | <del class="diffdel">Removed</del>
</small>
</p>
@ -17,17 +17,17 @@
<div class="byline">
<%= "Revision from #{@revision.pretty_created_at} by" %>
<%= "Revision from #{@revision.pretty_created_on} by" %>
<%= link_to_page @revision.author %>
</div>
<div class="navigation">
<% if @revision.next_revision %>
<% if @revision.next_revision.number < (@page.revisions.length - 1) %>
<% if @revision_number < @page.revisions.length - 1 %>
<% if @revision_number < @page.revisions.length - 2 %>
<%= link_to('Forward in time',
{:web => @web.address, :action => 'revision', :id => @page.name,
:rev => @revision.next_revision.number},
:rev => @revision_number + 1},
{:class => 'navlink', :name => 'to_next_revision'})
%>
<% else %>
@ -36,20 +36,20 @@
{:class => 'navlink', :name => 'to_next_revision'})
%>
<% end %>
<small>(<%= @revision.page.revisions.length - @revision.next_revision.number %> more)</small>
<small>(<%= @revision.page.revisions.length - @revision_number - 1 %> more)</small>
<% end %>
<% if @revision.next_revision && @revision.previous_revision %>
<% if @revision_number > 0 && @revision_number < @page.revisions.size - 1 %>
|
<% end %>
<% if @revision.previous_revision %>
<% if @revision_number > 0 %>
<%= link_to('Back in time',
{:web => @web.address, :action => 'revision', :id => @page.name,
:rev => @revision.previous_revision.number},
:rev => @revision_number - 1},
{:class => 'navlink', :name => 'to_previous_revision'})
%>
<small>(<%= @revision.previous_revision.number + 1 %> more)</small>
<small>(<%= @revision_number %> more)</small>
<% end %>
|
@ -57,7 +57,7 @@
{:class => 'navlink', :name => 'to_current_revision'})
%>
<% if @revision.previous_revision %>
<% if @revision_number > 0 %>
<span id="show_changes">
| <a href="#" onClick="toggleChanges(); return false;">See changes</a>
</span>
@ -69,7 +69,7 @@
|
<%= link_to('Rollback',
{:web => @web.address, :action => 'rollback', :id => @page.name, :rev => @revision.number},
{:web => @web.address, :action => 'rollback', :id => @page.name, :rev => @revision_number},
{:class => 'navlink', :name => 'rollback'})
%>

View file

@ -1,5 +1,5 @@
<%
@title = "Rollback to #{@page.plain_name} Rev ##{@revision.number}"
@title = "Rollback to #{@page.plain_name} Rev ##{@revision_number}"
@content_width = 720
@hide_navigation = true
%>

View file

@ -12,7 +12,7 @@
<% unless @hide_description %>
<description><%= h page.display_content %></description>
<% end %>
<pubDate><%= page.created_at.getgm.strftime "%a, %d %b %Y %H:%M:%S Z" %></pubDate>
<pubDate><%= page.revised_on.getgm.strftime "%a, %d %b %Y %H:%M:%S Z" %></pubDate>
<guid><%= url_for :only_path => false, :web => @web_name, :action => @link_action, :id => page.name %></guid>
<link><%= url_for :only_path => false, :web => @web_name, :action => @link_action, :id => page.name %></link>
<dc:creator><%= WikiWords.separate(page.author) %></dc:creator>

View file

@ -3,10 +3,14 @@ CREATE TABLE revisions (
created_at <%= @datetime %> NOT NULL,
updated_at <%= @datetime %> NOT NULL,
timestamp CHAR(13) NOT NULL,
-- note that continuous edits change the existing last revision, instead of creating one
-- revised_at is the timestamp of last edit of this revision
-- unlike updated_at is can be set explicitly in a call to Revision.create
-- besides, it should not be updated when Revision row changes for any other reason than
-- changing content
revised_at <%= @datetime %> NOT NULL,
page_id INTEGER NOT NULL,
content TEXT NOT NULL,
author VARCHAR(60),
ip VARCHAR(60),
number INTEGER
ip VARCHAR(60)
) <%= create_options %>;

View file

@ -2,9 +2,8 @@ home_page_first_revision:
id: 1
created_at: <%= Time.local(2004, 4, 4, 15, 50).to_formatted_s(:db) %>
updated_at: <%= Time.local(2004, 4, 4, 15, 50).to_formatted_s(:db) %>
timestamp: <%= (Time.local(2004, 4, 4, 15, 50).to_f * 1000).to_i %>
revised_at: <%= Time.local(2004, 4, 4, 15, 50).to_formatted_s(:db) %>
page_id: 1
number: 1
content: First revision of the HomePage end
author: AnAuthor
ip: 127.0.0.1
@ -13,9 +12,8 @@ my_way_first_revision:
id: 2
created_at: <%= 9.days.ago.to_formatted_s(:db) %>
updated_at: <%= 9.days.ago.to_formatted_s(:db) %>
timestamp: <%= (9.days.ago.to_f * 1000).to_i %>
revised_at: <%= 9.days.ago.to_formatted_s(:db) %>
page_id: 2
number: 1
content: MyWay
author: Me
@ -23,9 +21,8 @@ smart_engine_first_revision:
id: 3
created_at: <%= 8.days.ago.to_formatted_s(:db) %>
updated_at: <%= 8.days.ago.to_formatted_s(:db) %>
timestamp: <%= (8.days.ago.to_f * 1000).to_i %>
revised_at: <%= 8.days.ago.to_formatted_s(:db) %>
page_id: 3
number: 1
content: SmartEngine
author: Me
@ -33,9 +30,8 @@ that_way_first_revision:
id: 4
created_at: <%= 7.days.ago.to_formatted_s(:db) %>
updated_at: <%= 7.days.ago.to_formatted_s(:db) %>
timestamp: <%= (7.days.ago.to_f * 1000).to_i %>
revised_at: <%= 7.days.ago.to_formatted_s(:db) %>
page_id: 4
number: 1
content: ThatWay
author: Me
@ -43,9 +39,8 @@ no_wiki_word_first_revision:
id: 5
created_at: <%= 6.days.ago.to_formatted_s(:db) %>
updated_at: <%= 6.days.ago.to_formatted_s(:db) %>
timestamp: <%= (6.days.ago.to_f * 1000).to_i %>
revised_at: <%= 6.days.ago.to_formatted_s(:db) %>
page_id: 5
number: 1
content: hey you
author: Me
@ -53,9 +48,8 @@ home_page_second_revision:
id: 6
created_at: <%= Time.local(2004, 4, 4, 16, 50).to_formatted_s(:db) %>
updated_at: <%= Time.local(2004, 4, 4, 16, 50).to_formatted_s(:db) %>
timestamp: <%= (Time.local(2004, 4, 4, 16, 50).to_f * 1000).to_i %>
revised_at: <%= Time.local(2004, 4, 4, 16, 50).to_formatted_s(:db) %>
page_id: 1
number: 2
content: HisWay would be MyWay in kinda ThatWay in HisWay though MyWay \OverThere -- see SmartEngine in that SmartEngineGUI
author: DavidHeinemeierHansson
@ -63,9 +57,8 @@ first_page_first_revision:
id: 7
created_at: <%= Time.local(2004, 4, 4, 16, 55).to_formatted_s(:db) %>
updated_at: <%= Time.local(2004, 4, 4, 16, 55).to_formatted_s(:db) %>
timestamp: <%= (Time.local(2004, 4, 4, 16, 55).to_f * 1000).to_i %>
revised_at: <%= Time.local(2004, 4, 4, 16, 55).to_formatted_s(:db) %>
page_id: 6
number: 1
content: HisWay would be MyWay in kinda ThatWay in HisWay though MyWay \\OverThere -- see SmartEngine in that SmartEngineGUI
author: DavidHeinemeierHansson
@ -73,9 +66,8 @@ oak_first_revision:
id: 8
created_at: <%= 5.days.ago.to_formatted_s(:db) %>
updated_at: <%= 5.days.ago.to_formatted_s(:db) %>
timestamp: <%= (5.days.ago.to_f * 1000).to_i %>
revised_at: <%= 5.days.ago.to_formatted_s(:db) %>
page_id: 7
number: 1
content: "All about oak.\ncategory: trees"
author: TreeHugger
ip: 127.0.0.2
@ -84,9 +76,8 @@ elephant_first_revision:
id: 9
created_at: <%= 10.minutes.ago.to_formatted_s(:db) %>
updated_at: <%= 10.minutes.ago.to_formatted_s(:db) %>
timestamp: <%= (10.minutes.ago.to_f * 1000).to_i %>
revised_at: <%= 10.minutes.ago.to_formatted_s(:db) %>
page_id: 8
number: 1
content: "All about elephants.\ncategory: animals"
author: Guest
ip: 127.0.0.2

View file

@ -104,7 +104,7 @@ class WikiControllerTest < Test::Unit::TestCase
end
def test_export_html
@home.rollback(1, Time.now, 'Rick') # much simpler regex statement to match
@home.rollback(0, Time.now, 'Rick') # much simpler regex statement to match
r = process 'export_html', 'web' => 'wiki1'
assert_response :success
@ -355,7 +355,7 @@ class WikiControllerTest < Test::Unit::TestCase
assert_response :success
assert_equal @home, r.template_objects['page']
assert_equal @home.revisions[0], r.template_objects['revision']
assert_equal @home.revisions[0], r.template_objects['revision']
end

View file

@ -32,27 +32,37 @@ class PageTest < Test::Unit::TestCase
def test_revise
@page.revise('HisWay would be MyWay in kinda lame', Time.local(2004, 4, 4, 16, 55), 'MarianneSyhler')
assert_equal 2, @page.revisions(true).length, 'Should have two revisions'
assert_equal 'MarianneSyhler', @page.current_revision(true).author.to_s, 'Mary should be the author now'
@page.reload
assert_equal 2, @page.revisions.length, 'Should have two revisions'
assert_equal 'MarianneSyhler', @page.current_revision.author.to_s, 'Mary should be the author now'
assert_equal 'DavidHeinemeierHansson', @page.revisions.first.author.to_s, 'David was the first author'
end
def test_revise_continous_revision
@page.revise('HisWay would be MyWay in kinda lame', Time.local(2004, 4, 4, 16, 55), 'MarianneSyhler')
assert_equal 2, @page.revisions(true).length
@page.reload
assert_equal 2, @page.revisions.length
assert_equal 'HisWay would be MyWay in kinda lame', @page.content
@page.current_revision(true)
# consecutive revision by the same author within 30 minutes doesn't create a new revision
@page.revise('HisWay would be MyWay in kinda update', Time.local(2004, 4, 4, 16, 57), 'MarianneSyhler')
assert_equal 2, @page.revisions(true).length
assert_equal 'HisWay would be MyWay in kinda update', @page.revisions.last.content
assert_equal Time.local(2004, 4, 4, 16, 57), @page.revisions.last.created_at
@page.reload
assert_equal 2, @page.revisions.length
assert_equal 'HisWay would be MyWay in kinda update', @page.content
assert_equal Time.local(2004, 4, 4, 16, 57), @page.revised_on
# but consecutive revision by another author results in a new revision
@page.revise('HisWay would be MyWay in the house', Time.local(2004, 4, 4, 16, 58), 'DavidHeinemeierHansson')
assert_equal 3, @page.revisions(true).length
assert_equal 'HisWay would be MyWay in the house', @page.revisions.last.content
@page.reload
assert_equal 3, @page.revisions.length
assert_equal 'HisWay would be MyWay in the house', @page.content
# consecutive update after 30 minutes since the last one also creates a new revision,
# even when it is by the same author
@page.revise('HisWay would be MyWay in my way', Time.local(2004, 4, 4, 17, 30), 'DavidHeinemeierHansson')
assert_equal 4, @page.revisions(true).length
@page.reload
assert_equal 4, @page.revisions.length
end
def test_revise_content_unchanged
@ -72,7 +82,7 @@ class PageTest < Test::Unit::TestCase
@page.revise("spot three", Time.now + 2000, "David")
assert_equal 3, @page.revisions(true).length, "Should have three revisions"
@page.current_revision(true)
@page.rollback(1, Time.now)
@page.rollback(0, Time.now)
assert_equal "HisWay would be MyWay in kinda ThatWay in HisWay though MyWay \\\\OverThere -- see SmartEngine in that SmartEngineGUI", @page.current_revision(true).content
end
end

View file

@ -245,8 +245,10 @@ class RevisionTest < Test::Unit::TestCase
end
def test_revisions_diff
Revision.create(:page => @page, :content => 'What a blue and lovely morning', :author => Author.new('DavidHeinemeierHansson'))
Revision.create(:page => @page, :content => 'What a red and lovely morning today', :author => Author.new('DavidHeinemeierHansson'))
Revision.create(:page => @page, :content => 'What a blue and lovely morning',
:author => Author.new('DavidHeinemeierHansson'), :revised_at => Time.now)
Revision.create(:page => @page, :content => 'What a red and lovely morning today',
:author => Author.new('DavidHeinemeierHansson'), :revised_at => Time.now)
assert_equal "<p>What a <del class=\"diffmod\">blue </del><ins class=\"diffmod\">red " +
"</ins>and lovely <del class=\"diffmod\">morning</del><ins class=\"diffmod\">morning " +

View file

@ -248,7 +248,7 @@ class E2EInstikiTest < Test::Unit::TestCase
def date_pattern
'(January|February|March|April|May|June|July|August|September|October|November|December) ' +
'\d\d?, \d\d\d\d \d\d:\d\d'
'\d\d?, \d\d\d\d \d\d:\d\d:\d\d'
end
def enter_markup(page, content, author = nil)