re add scope (use in a f**ckin eval expression that we need to remove)

remove some url that no more used improve some code readability around url methods
publify · Dec 9, 2012 · 3bba314 · 3bba314
1 parent e46ac7c
commit 3bba314
Show file tree

Hide file tree

Showing 9 changed files with 58 additions and 147 deletions.
diff --git a/app/controllers/xml_controller.rb b/app/controllers/xml_controller.rb
@@ -22,7 +22,7 @@ def feed
     when 'comments'
       redirect_to admin_comments_url(format: @format), status: :moved_permanently
     when 'article'
-      redirect_to Article.find(params[:id]).permalink_by_format(@format), status: :moved_permanently
+      redirect_to Article.find(params[:id]).feed_url(@format), status: :moved_permanently
     when 'category', 'tag', 'author'
       redirect_to self.send("#{params[:type]}_url", params[:id], format: @format), status: :moved_permanently
     when 'trackbacks'
@@ -50,7 +50,7 @@ def feed
 
   # TODO: Move redirects into config/routes.rb, if possible
   def articlerss
-    redirect_to Article.find(params[:id]).permalink_by_format('rss'), status: :moved_permanently
+    redirect_to Article.find(params[:id]).feed_url('rss'), status: :moved_permanently
   end
 
   def commentrss

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
@@ -216,7 +216,7 @@ def feed_for(type)
     if params[:action] == 'search'
       url_for(only_path: false, format: type, q: params[:q])
     elsif not @article.nil?
-      @article.feed_url(type.to_sym)
+      @article.feed_url(type)
     elsif not @auto_discovery_url_atom.nil?
       instance_variable_get("@auto_discovery_url_#{type}")
     end

diff --git a/app/models/article.rb b/app/models/article.rb
@@ -57,6 +57,19 @@ def spam
 
   setting :password, :string, ''
 
+  attr_accessor :draft, :keywords
+
+  include Article::States
+
+  has_state(:state, :valid_states  => [:new, :draft,
+                                       :publication_pending, :just_published, :published,
+                                       :just_withdrawn, :withdrawn],
+                                       :initial_state =>  :new,
+                                       :handles       => [:withdraw,
+                                                          :post_trigger,
+                                                          :send_pings, :send_notifications,
+                                                          :published_at=, :just_published?])
+
   def initialize(*args)
     super
     # Yes, this is weird - PDC
@@ -78,23 +91,9 @@ def set_author(user)
   end
 
   def has_child?
-    Article.exists?({:parent_id => self.id})
+    Article.exists?(parent_id: self.id)
   end
 
-  attr_accessor :draft, :keywords
-
-  has_state(:state,
-            :valid_states  => [:new, :draft,
-                               :publication_pending, :just_published, :published,
-                               :just_withdrawn, :withdrawn],
-                               :initial_state =>  :new,
-                               :handles       => [:withdraw,
-                                                  :post_trigger,
-                                                  :send_pings, :send_notifications,
-                                                  :published_at=, :just_published?])
-
-  include Article::States
-
   def self.last_draft(article_id)
     article = Article.find(article_id)
     while article.has_child?
@@ -118,28 +117,12 @@ def self.search_with_pagination(search_hash, paginate_hash)
     eval(list_function.join('.'))
   end
 
-  def year_url
-    published_at.year.to_s
-  end
-
-  def month_url
-    sprintf("%.2d", published_at.month)
-  end
-
-  def day_url
-    sprintf("%.2d", published_at.day)
-  end
-
-  def title_url
-    URI.encode(permalink.to_s)
-  end
-
-  def permalink_url_options(nesting = false)
+  def permalink_url_options
     format_url = blog.permalink_format.dup
-    format_url.gsub!('%year%', year_url)
-    format_url.gsub!('%month%', month_url)
-    format_url.gsub!('%day%', day_url)
-    format_url.gsub!('%title%', title_url)
+    format_url.gsub!('%year%', published_at.year.to_s)
+    format_url.gsub!('%month%', sprintf("%.2d", published_at.month))
+    format_url.gsub!('%day%', sprintf("%.2d", published_at.day))
+    format_url.gsub!('%title%', URI.encode(permalink.to_s))
     if format_url[0,1] == '/'
       format_url[1..-1]
     else
@@ -149,9 +132,7 @@ def permalink_url_options(nesting = false)
 
   def permalink_url(anchor=nil, only_path=false)
     @cached_permalink_url ||= {}
-
-    @cached_permalink_url["#{anchor}#{only_path}"] ||= \
-      blog.url_for(permalink_url_options, :anchor => anchor, :only_path => only_path)
+    @cached_permalink_url["#{anchor}#{only_path}"] ||= blog.url_for(permalink_url_options, anchor: anchor, only_path: only_path)
   end
 
   def save_attachments!(files)
@@ -169,18 +150,6 @@ def trackback_url
     blog.url_for("trackbacks?article_id=#{self.id}", :only_path => false)
   end
 
-  def permalink_by_format(format=nil)
-    if format.nil?
-      permalink_url
-    elsif format.to_sym == :rss
-      feed_url(:rss)
-    elsif format.to_sym == :atom
-      feed_url(:atom)
-    else
-      raise UnSupportedFormat
-    end
-  end
-
   def comment_url
     blog.url_for("comments?article_id=#{self.id}", :only_path => false)
   end
@@ -189,28 +158,8 @@ def preview_comment_url
     blog.url_for("comments/preview?article_id=#{self.id}", :only_path => false)
   end
 
-  def feed_url(format = :rss)
-    format_extension = format.to_s.gsub(/\d/,'')
-    permalink_url + ".#{format_extension}"
-  end
-
-  def edit_url
-    blog.url_for(:controller => "/admin/content", :action =>"edit", :id => id)
-  end
-
-  def delete_url
-    blog.url_for(:controller => "/admin/content", :action =>"destroy", :id => id)
-  end
-
-  def html_urls
-    urls = Array.new
-    html.gsub(/<a\s+[^>]*>/) do |tag|
-      if(tag =~ /\bhref=(["']?)([^ >"]+)\1/)
-        urls.push($2.strip)
-      end
-    end
-
-    urls.uniq
+  def feed_url(format)
+    "#{permalink_url}.#{format.gsub(/\d/,'')}"
   end
 
   def really_send_pings
@@ -238,23 +187,11 @@ def really_send_pings
   end
 
   def next
-    self.class.find(:first, :conditions => ['published_at > ?', published_at],
-                    :order => 'published_at asc')
+    Article.where('published_at > ?', published_at).order('published_at asc').limit(1).first
   end
 
   def previous
-    self.class.find(:first, :conditions => ['published_at < ?', published_at],
-                    :order => 'published_at desc')
-  end
-
-  # Count articles on a certain date
-  def self.count_by_date(year, month = nil, day = nil, limit = nil)
-    if !year.blank?
-      count(:conditions => { :published_at => time_delta(year, month, day),
-                             :published => true })
-    else
-      count(:conditions => { :published => true })
-    end
+    Article.where('published_at < ?', published_at).order('published_at desc').limit(1).first
   end
 
   def self.find_by_published_at
@@ -367,6 +304,17 @@ def comments_closed?
     !(allow_comments? && in_feedback_window?)
   end
 
+  def html_urls
+    urls = Array.new
+    html.gsub(/<a\s+[^>]*>/) do |tag|
+      if(tag =~ /\bhref=(["']?)([^ >"]+)\1/)
+        urls.push($2.strip)
+      end
+    end
+    urls.uniq
+  end
+
+
   def pings_closed?
     !(allow_pings? && in_feedback_window?)
   end
@@ -472,7 +420,7 @@ def self.time_delta(year = nil, month = nil, day = nil)
 
   def html_urls_to_ping
     urls_to_ping = []
-    self.html_urls.delete_if{|url| already_ping?(url)}.uniq.each do |url_to_ping|
+    html_urls.delete_if{|url| already_ping?(url)}.uniq.each do |url_to_ping|
       urls_to_ping << self.pings.build("url" => url_to_ping)
     end
     urls_to_ping

diff --git a/app/models/feedback.rb b/app/models/feedback.rb
@@ -48,14 +48,6 @@ def permalink_url(anchor=:ignored, only_path=false)
     article.permalink_url("#{self.class.to_s.downcase}-#{id}",only_path)
   end
 
-  def edit_url(anchor=:ignored)
-    blog.url_for(:controller => "/admin/#{self.class.to_s.downcase}s", :action =>"edit", :id => id)
-  end
-
-  def delete_url(anchor=:ignored)
-    blog.url_for(:controller => "/admin/#{self.class.to_s.downcase}s", :action =>"destroy", :id => id)
-  end
-
   def html_postprocess(field, html)
     helper = ContentTextHelpers.new
     helper.sanitize(helper.auto_link(html)).nofollowify

diff --git a/app/views/articles/read.html.erb b/app/views/articles/read.html.erb
@@ -46,7 +46,7 @@
 
 <p class="postmetadata alt">
   <small>
-  <a href="<%= @article.feed_url(:rss) %>" title="RSS Feed"><%= _("RSS feed for this post")%></a>
+  <a href="<%= @article.feed_url('rss') %>" title="RSS Feed"><%= _("RSS feed for this post")%></a>
 <% if @article.allow_pings? -%>
   <a href="<%= @article.trackback_url %>" ><%= _("trackback uri")%></a>
 <% end -%>

diff --git a/spec/controllers/articles_controller_spec.rb b/spec/controllers/articles_controller_spec.rb
@@ -569,17 +569,6 @@
       @article = FactoryGirl.create(:article)
     end
 
-    it "should find the article if the url matches all components" do
-      get :redirect, :from => "foo/bar/#{@article.year_url}/#{@article.month_url}/#{@article.permalink}"
-      response.should be_success
-    end
-
-    # FIXME: Documents current behavior; Blog URL format is only meant for one article shown
-    it "should not find the article if the url only matches some components" do
-      get :redirect, :from => "foo/bar/#{@article.year_url}/#{@article.month_url}"
-      assert_response 404
-    end
-
     # TODO: Think about allowing this, and changing find_by_params_hash to match.
     if false
       it "should find the article if the url matches all fixed parts and no variable components" do

diff --git a/spec/controllers/xml_controller_spec.rb b/spec/controllers/xml_controller_spec.rb
@@ -110,21 +110,21 @@ def assert_moved_permanently_to(location)
       describe "without format parameter" do
         it "redirects article feed to Article RSS feed" do
           get :feed, :type => 'article', :id => @article.id
-          assert_moved_permanently_to @article.permalink_by_format(:rss)
+          assert_moved_permanently_to @article.feed_url('rss')
         end
       end
 
       describe "with format rss20" do
         it "redirects the article feed to the article RSS feed" do
           get :feed, :format => 'rss20', :type => 'article', :id => @article.id
-          assert_moved_permanently_to @article.permalink_by_format(:rss)
+          assert_moved_permanently_to @article.feed_url('rss')
         end
       end
 
       describe "with format atom10" do
         it "redirects the article feed to the article Atom feed" do
           get :feed, :format => 'atom10', :type => 'article', :id => @article.id
-          assert_moved_permanently_to @article.permalink_by_format('atom')
+          assert_moved_permanently_to @article.feed_url('atom')
         end
       end
     end
@@ -148,7 +148,7 @@ def assert_moved_permanently_to(location)
 
     it "redirects permanently to the article RSS feed" do
       get :articlerss, :id => @article.id
-      assert_moved_permanently_to @article.permalink_by_format(:rss)
+      assert_moved_permanently_to @article.feed_url('rss')
     end
   end
 

diff --git a/spec/models/article_spec.rb b/spec/models/article_spec.rb
@@ -61,20 +61,16 @@ def assert_results_are(*expected)
     end
   end
 
-  it "test_edit_url" do
-    a = stub_model(Article, :id => 123)
-    assert_equal "http://myblog.net/admin/content/edit/#{a.id}", a.edit_url
-  end
+  describe ".feed_url" do
+    let(:article) { FactoryGirl.build(:article, permalink: 'article-3', published_at: Time.utc(2004, 6, 1)) }
 
-  it "test_delete_url" do
-    a = stub_model(Article, :id => 123)
-    assert_equal "http://myblog.net/admin/content/destroy/#{a.id}", a.delete_url
-  end
+    it "returns url for atom feed for a Atom 1.0 asked" do
+      article.feed_url('atom10').should eq "http://myblog.net/2004/06/01/article-3.atom"
+    end
 
-  it "test_feed_url" do
-    a = stub_model(Article, :permalink => 'article-3', :published_at => Time.utc(2004, 6, 1))
-    assert_equal "http://myblog.net/2004/06/01/article-3.atom", a.feed_url(:atom10)
-    assert_equal "http://myblog.net/2004/06/01/article-3.rss", a.feed_url(:rss20)
+    it "returns url for rss feed for a RSS 2 asked" do
+      article.feed_url('rss20').should eq "http://myblog.net/2004/06/01/article-3.rss"
+    end
   end
 
   it "test_create" do
@@ -94,7 +90,7 @@ def assert_results_are(*expected)
   it "test_permalink_with_title" do
     article = FactoryGirl.create(:article, :permalink => 'article-3', :published_at => Time.utc(2004, 6, 1))
     assert_equal(article,
-                Article.find_by_permalink({:year => 2004, :month => 06, :day => 01, :title => "article-3"}) )
+                 Article.find_by_permalink({:year => 2004, :month => 06, :day => 01, :title => "article-3"}) )
     assert_raises(ActiveRecord::RecordNotFound) do
       Article.find_by_permalink :year => 2005, :month => "06", :day => "01", :title => "article-5"
     end
@@ -178,25 +174,25 @@ def assert_results_are(*expected)
   ### XXX: Should we have a test here?
   it "test_send_multiple_pings" do
   end
-  
+
   describe "Testing redirects" do
     it "a new published article gets a redirect" do
       a = Article.create(:title => "Some title", :body => "some text", :published => true)
       a.redirects.first.should_not be_nil
       a.redirects.first.to_path.should == a.permalink_url
     end
-    
+
     it "a new unpublished article should not get a redirect" do 
       a = Article.create(:title => "Some title", :body => "some text", :published => false)
       a.redirects.first.should be_nil
     end
-    
+
     it "Changin a published article permalink url should only change the to redirection" do
       a = Article.create(:title => "Some title", :body => "some text", :published => true)
       a.redirects.first.should_not be_nil
       a.redirects.first.to_path.should == a.permalink_url
       r  = a.redirects.first.from_path
-      
+
       a.permalink = "some-new-permalink"
       a.save
       a.redirects.first.should_not be_nil
@@ -269,7 +265,7 @@ def assert_results_are(*expected)
 
   it "test_future_publishing" do
     assert_sets_trigger(Article.create!(:title => 'title', :body => 'body',
-      :published => true, :published_at => Time.now + 4.seconds))
+                                        :published => true, :published_at => Time.now + 4.seconds))
   end
 
   it "test_future_publishing_without_published_flag" do

diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb
@@ -28,20 +28,6 @@ def valid_comment(options={})
     end
   end
 
-  describe '#edit_url' do
-    it 'should get a url where edit comment in admin' do
-      c = FactoryGirl.build_stubbed(:comment)
-      assert_equal "http://myblog.net/admin/comments/edit/#{c.id}", c.edit_url
-    end
-  end
-
-  describe '#delete_url' do
-    it 'should get the delete url of comment in admin part' do
-      c = FactoryGirl.build_stubbed(:comment)
-      assert_equal "http://myblog.net/admin/comments/destroy/#{c.id}", c.delete_url
-    end
-  end
-
   describe '#save' do
     before(:each) {
       @blog.stub(:sp_article_auto_close) { 300 }