Refactoring page creation code [dm1]

app/models/page.rb

@@ -10,11 +10,10 @@ class Page
   attr_reader :name, :web
   attr_accessor :revisions
   
-  def initialize(web, name, content, created_at, author)
+  def initialize(web, name)
     raise 'nil web' if web.nil?
     raise 'nil name' if name.nil?
     @web, @name, @revisions = web, name, []
-    revise(content, created_at, author)
   end
 
   def revise(content, created_at, author)
@@ -43,6 +42,9 @@ class Page
     self.revisions.last.clear_display_cache
     
     @web.refresh_pages_with_references(@name) if @revisions.length == 1
+    
+    self
+    
   end
 
   def rollback(revision_number, created_at, author_ip = nil)

app/models/web.rb

@@ -44,10 +44,12 @@ class Web
   def max_upload_size() @max_upload_size || 100; end
   def wiki() @wiki ||= WikiService.instance; end
 
-  def add_page(page)
+   def add_page(name, content, created_at, author)
-    @pages[page.name] = page
+     page = Page.new(self, name)
-  end
+     @pages[page.name] = page
-
+     page.revise(content, created_at, author)
+   end
+  
   def address=(the_address)
     if the_address != CGI.escape(the_address)
       raise Instiki::ValidationError.new('Web name should contain only valid URI characters') 

app/models/wiki_service.rb

@@ -58,13 +58,11 @@ module AbstractWikiService
   def revise_page(web_address, page_name, content, revised_on, author)
     page = read_page(web_address, page_name)
     page.revise(content, revised_on, author)
-    page
   end
 
   def rollback_page(web_address, page_name, revision_number, created_at, author_id = nil)
     page = read_page(web_address, page_name)
     page.rollback(revision_number, created_at, author_id)
-    page
   end
   
   def setup(password, web_name, web_address)
@@ -105,9 +103,7 @@ module AbstractWikiService
   end
 
   def write_page(web_address, page_name, content, written_on, author)
-    page = Page.new(@webs[web_address], page_name, content, written_on, author)
+    @webs[web_address].add_page(page_name, content, written_on, author)
-    @webs[web_address].add_page(page)
-    page
   end
 
   def storage_path

test/unit/page_test.rb

@@ -13,10 +13,8 @@ class PageTest < Test::Unit::TestCase
   end
 
   def setup
-    @page = Page.new(
+    @page = Page.new(MockWeb.new, "FirstPage")
-      MockWeb.new,
+    @page.revise("HisWay would be MyWay in kinda ThatWay in HisWay though MyWay \\OverThere -- see SmartEngine in that SmartEngineGUI", 
-      "FirstPage", 
-      "HisWay would be MyWay in kinda ThatWay in HisWay though MyWay \\OverThere -- see SmartEngine in that SmartEngineGUI", 
       Time.local(2004, 4, 4, 16, 50),
       "DavidHeinemeierHansson")
   end

test/unit/web_test.rb

@@ -9,15 +9,15 @@ class WebTest < Test::Unit::TestCase
   end
   
   def test_wiki_word_linking
-    @web.add_page(Page.new(@web, 'SecondPage', 'Yo, yo. Have you EverBeenHated', Time.now, 
+    @web.add_page('SecondPage', 'Yo, yo. Have you EverBeenHated', 
-        'DavidHeinemeierHansson'))
+                   Time.now, 'DavidHeinemeierHansson')
     
     assert_equal('<p>Yo, yo. Have you <span class="newWikiWord">Ever Been Hated' + 
         '<a href="../show/EverBeenHated">?</a></span></p>', 
     @web.pages["SecondPage"].display_content)
     
-    @web.add_page(Page.new(@web, 'EverBeenHated', 'Yo, yo. Have you EverBeenHated', Time.now, 
+    @web.add_page('EverBeenHated', 'Yo, yo. Have you EverBeenHated', Time.now, 
-        'DavidHeinemeierHansson'))
+                  'DavidHeinemeierHansson')
     assert_equal('<p>Yo, yo. Have you <a class="existingWikiWord" ' +
         'href="../show/EverBeenHated">Ever Been Hated</a></p>', 
     @web.pages['SecondPage'].display_content)
@@ -128,28 +128,25 @@ class WebTest < Test::Unit::TestCase
 
   def test_new_page_linked_from_mother_page
     # this was a bug in revision 204
-    home = Page.new(@web, 'HomePage', 'This page refers to AnotherPage', 
+    home = @web.add_page('HomePage', 'This page refers to AnotherPage', 
         Time.local(2004, 4, 4, 16, 50), 'Alexey Verkhovsky')
-    another_page = Page.new(@web, 'AnotherPage', 'This is \AnotherPage', 
+    @web.add_page('AnotherPage', 'This is \AnotherPage', 
         Time.local(2004, 4, 4, 16, 51), 'Alexey Verkhovsky')
-
-    @web.add_page(home)
-    @web.add_page(another_page)
     
     assert_equal [home], @web.select.pages_that_link_to('AnotherPage')
   end
 
   def test_orphaned_pages
     add_sample_pages
-    home = @web.add_page(Page.new(@web, 'HomePage', 
+    home = @web.add_page('HomePage', 
         'This is a home page, it should not be an orphan',
-        Time.local(2004, 4, 4, 16, 50), 'AlexeyVerkhovsky'))
+        Time.local(2004, 4, 4, 16, 50), 'AlexeyVerkhovsky')
-    author = @web.add_page(Page.new(@web, 'AlexeyVerkhovsky', 
+    author = @web.add_page('AlexeyVerkhovsky', 
         'This is an author page, it should not be an orphan',
-        Time.local(2004, 4, 4, 16, 50), 'AlexeyVerkhovsky'))
+        Time.local(2004, 4, 4, 16, 50), 'AlexeyVerkhovsky')
-    self_linked = @web.add_page(Page.new(@web, 'SelfLinked', 
+    self_linked = @web.add_page('SelfLinked', 
         'I am me SelfLinked and link to EverBeenInLove',
-        Time.local(2004, 4, 4, 16, 50), 'AnonymousCoward'))
+        Time.local(2004, 4, 4, 16, 50), 'AnonymousCoward')
         
     # page that links to itself, and nobody else links to it must be an orphan
     assert_equal ['EverBeenHated', 'SelfLinked'], 
@@ -160,9 +157,9 @@ class WebTest < Test::Unit::TestCase
   private
 
   def add_sample_pages
-    @in_love = @web.add_page(Page.new(@web, 'EverBeenInLove', 'Who am I me', 
+    @in_love = @web.add_page('EverBeenInLove', 'Who am I me', 
-        Time.local(2004, 4, 4, 16, 50), 'DavidHeinemeierHansson'))
+        Time.local(2004, 4, 4, 16, 50), 'DavidHeinemeierHansson')
-    @hated = @web.add_page(Page.new(@web, 'EverBeenHated', 'I am me EverBeenHated', 
+    @hated = @web.add_page('EverBeenHated', 'I am me EverBeenHated', 
-        Time.local(2004, 4, 4, 16, 51), 'DavidHeinemeierHansson'))
+        Time.local(2004, 4, 4, 16, 51), 'DavidHeinemeierHansson')
   end
 end