Removing eval in content, page and article class

app/controllers/admin/content_controller.rb

@@ -14,14 +14,12 @@ def auto_complete_for_article_keywords
 
   def index
     @search = params[:search] ? params[:search] : {}
-
-    @articles = Article.search_with_pagination(@search, {page: params[:page], per_page: this_blog.admin_display_elements})
+    @articles = Article.search_with(@search).page(params[:page]).per(this_blog.admin_display_elements)
 
     if request.xhr?
       render partial: 'article_list', locals: { articles: @articles }
     else
       @article = Article.new(params[:article])
-
     end
   end
 

app/controllers/admin/pages_controller.rb

@@ -11,7 +11,7 @@ class Admin::PagesController < Admin::BaseController
 
   def index
     @search = params[:search] ? params[:search] : {}
-    @pages = Page.search_paginate(@search, :page => params[:page], :per_page => this_blog.admin_display_elements)
+    @pages = Page.search_with(@search).page(params[:page]).per(this_blog.admin_display_elements)
   end
 
   def new

app/helpers/admin/base_helper.rb

@@ -80,21 +80,25 @@ def task_overview
     content_tag :li, link_to(_('Back to list'), :action => 'index')
   end
 
-  def render_void_table(size, cols)
-    return unless size == 0
+  def render_empty_table(cols)
     content_tag(:tr) do
-      content_tag(:td, _("There are no %s yet. Why don't you start and create one?", _(controller.controller_name)), { :colspan => cols})
+      content_tag(:td, _("There are no %s yet. Why don't you start and create one?", _(controller.controller_name)), { colspan: cols})
     end
   end
 
+  def render_void_table(size, cols)
+    return unless size == 0
+    render_empty_table(cols)
+  end
+
   def cancel_or_save(message=_("Save"))
     "#{cancel} #{_("or")} #{save(message)}"
   end
 
-    def get_short_url(item)
-      return "" if item.short_url.nil?
-      sprintf(content_tag(:small, "%s %s"), _("Short url:"), link_to(item.short_url, item.short_url))
-    end
+  def get_short_url(item)
+    return "" if item.short_url.nil?
+    sprintf(content_tag(:small, "%s %s"), _("Short url:"), link_to(item.short_url, item.short_url))
+  end
 
   def show_actions item
     content_tag(:div, { :class => 'action', :style => '' }) do
@@ -156,12 +160,12 @@ def render_macros(macros)
 
   def build_editor_link(label, action, id, update, editor)
     link = link_to_remote(label,
-            :url => { :action => action, 'editor' => editor},
-            :method => :get,
-            :class => 'ui-button-text',
-            :loading => "new Element.show('update_spinner_#{id}')",
-            :success => "new Element.toggle('update_spinner_#{id}')",
-            :update => "#{update}")
+                          :url => { :action => action, 'editor' => editor},
+                          :method => :get,
+                          :class => 'ui-button-text',
+                          :loading => "new Element.show('update_spinner_#{id}')",
+                          :success => "new Element.toggle('update_spinner_#{id}')",
+                          :update => "#{update}")
     link << image_tag("spinner-blue.gif", :id => "update_spinner_#{id}", :style => 'display:none;')
   end
 

app/models/article.rb

@@ -113,23 +113,18 @@ def self.last_draft(article_id)
     article
   end
 
-  def self.search_with_pagination(search_hash, paginate_hash)
-    state = (search_hash[:state] and ["no_draft", "drafts", "published", "withdrawn", "pending"].include? search_hash[:state]) ? search_hash[:state] : nil
-
-    if state.nil?
-      list_function  = function_search_all_posts(search_hash)
-    elsif
-      list_function  = ["Article.#{state}"] + function_search_all_posts(search_hash)
+  def self.search_with(params)
+    params ||= {}
+    scoped = super(params)
+    if ["no_draft", "drafts", "published", "withdrawn", "pending"].include?(params[:state])
+      scoped = scoped.send(params[:state])
     end
 
-    if search_hash[:category] && search_hash[:category].to_i > 0
-      list_function << 'category(search_hash[:category])'
+    if params[:category] && params[:category].to_i > 0
+      scoped = scoped.category(params[:category])
     end
 
-    list_function << "page(paginate_hash[:page])"
-    list_function << "per(paginate_hash[:per_page])"
-    list_function << "order('published_at desc, created_at desc')"
-    eval(list_function.join('.'))
+    scoped.order('published_at DESC').order('created_at DESC')
   end
 
   def permalink_url(anchor=nil, only_path=false)

app/models/content.rb

@@ -30,7 +30,6 @@ class Content < ActiveRecord::Base
   }
   scope :already_published, lambda { where('published = ? AND published_at < ?', true, Time.now).order(default_order) }
 
-  # Use only for self.function_search_all_posts method
   scope :published_at_like, lambda { |date_at| where(:published_at => (
       if date_at =~ /\d{4}-\d{2}-\d{2}/
         DateTime.strptime(date_at, '%Y-%m-%d').beginning_of_day..DateTime.strptime(date_at, '%Y-%m-%d').end_of_day
@@ -67,28 +66,27 @@ def self.find_already_published(limit)
     where('published_at < ?', Time.now).limit(1000).order('created_at DESC')
   end
 
-  def self.function_search_all_posts(search_hash)
-    list_function = []
-    search_hash ||= {}
-
-    if search_hash[:searchstring]
-      list_function << 'searchstring(search_hash[:searchstring])' unless search_hash[:searchstring].to_s.empty?
+  def self.search_with(params)
+    params ||= {}
+    scoped = self.unscoped
+    if params[:searchstring].present?
+      scoped = scoped.searchstring(params[:searchstring])
     end
 
-    if search_hash[:published_at] and %r{(\d\d\d\d)-(\d\d)} =~ search_hash[:published_at]
-      list_function << 'published_at_like(search_hash[:published_at])'
+    if params[:published_at].present? && %r{(\d\d\d\d)-(\d\d)} =~ params[:published_at]
+      scoped = scoped.published_at_like(params[:published_at])
     end
 
-    if search_hash[:user_id] && search_hash[:user_id].to_i > 0
-      list_function << 'user_id(search_hash[:user_id])'
+    if params[:user_id].present? && params[:user_id].to_i > 0
+      scoped = scoped.user_id(params[:user_id])
     end
 
-    if search_hash[:published]
-      list_function << 'published' if search_hash[:published].to_s == '1'
-      list_function << 'not_published' if search_hash[:published].to_s == '0'
+    if params[:published].present?
+      scoped = scoped.published if params[:published].to_s == '1'
+      scoped = scoped.not_published if params[:published].to_s == '0'
     end
 
-    list_function
+    scoped
   end
 
   def whiteboard

app/models/page.rb

@@ -31,12 +31,8 @@ def self.default_order
     'name ASC'
   end
 
-  def self.search_paginate(search_hash, paginate_hash)
-    list_function = ["Page"] + function_search_all_posts(search_hash)
-    paginate_hash[:order] = 'title ASC'
-    list_function << "page(paginate_hash[:page])"
-    list_function << "per(paginate_hash[:per_page])"
-    eval(list_function.join('.'))
+  def self.search_with(search_hash)
+    super(search_hash).order('title ASC')
   end
 
   def permalink_url(anchor=nil, only_path=false)

app/views/admin/content/_article_list.html.erb

@@ -1,4 +1,4 @@
-<%= render_void_table(@articles.size, 7)  %>
+<%= render_empty_table(7) if @articles.empty? %>
 <% for article in @articles %>
 <tr <%= alternate_class %>>
   <td class='article-title'>

app/views/admin/content/index.html.erb

@@ -1,40 +1,33 @@
 <% @page_heading = _('Manage articles') + content_tag(:div, link_to(_("New Article"), {:controller => 'content', :action => 'new'}, :id => 'dialog-link', :class => 'btn btn-info'), :class => 'pull-right') %>
 
-<%= form_remote_tag  \
-  :url      => {:action => 'index'},
-  :method   => :get,
-  :name     => 'article',
-  :update   => {:success => 'articleList'},
-  :before   => "Element.show('spinner')",
-  :complete => "Element.hide('spinner')" \
-  do %>
+<%= form_remote_tag url: {action: 'index'}, method: :get, name: 'article', update: {:success => 'articleList'}, before: "Element.show('spinner')", complete: "Element.hide('spinner')" do %>
 
 <% if params[:search] and params[:search]['state'] %>
 <input type='hidden' name="search[state]" value="<%= params[:search]['state'] %>" >
 <% end %>
 
 <div>
   <span class='badge badge-inverse'>
-    <%= link_to(_("All articles"), :action => 'index') %>
+    <%= link_to(_("All articles"), action: 'index') %>
   </span>
   <span class='badge badge-success'>
-    <%= link_to(_("Published"), :action => 'index', :search => {:state => 'published'}) %>
+    <%= link_to(_("Published"), action: 'index', search: {state: 'published'}) %>
   </span>
   <span class='badge badge-important'>
-    <%= link_to(_("Withdrawn"), :action => 'index', :search => {:state => 'withdrawn'}) %>
+    <%= link_to(_("Withdrawn"), action: 'index', search: {state: 'withdrawn'}) %>
   </span>
   <span class='badge badge-info'>
-    <%= link_to(_("Drafts"), :action => 'index', :search => {:state => 'drafts'}) %>
+    <%= link_to(_("Drafts"), action: 'index', search: {state: 'drafts'}) %>
   </span>
   <span class='badge badge-warning'>
-    <%= link_to(_("Publication pending"), :action => 'index', :search => {:state => 'pending'}) %>
+    <%= link_to(_("Publication pending"), action: 'index', search: {state: 'pending'}) %>
   </span>
   <div class='pull-right'>
     <span class="input-append">
       <input id="search" type="text" name="search[searchstring]" class='large' />
       <%= submit_tag(_("Search"), {:class => 'btn'}) %>
       <span id='spinner' style="display:none;"><%= image_tag('spinner.gif') %></span>
-    </span>    
+    </span>
   </div>
 </div>
 <br class='clearleft' />
@@ -53,7 +46,7 @@
       <th><%= _("Author")%></th>
       <th><%= _("Date") %></th>
       <th class='last'><%= _("Feedback")%></th>
-    </tr>    
+    </tr>
   </thead>
   <tbody id="articleList">
     <%= render 'article_list', { :articles => @articles } -%>

spec/controllers/admin/content_controller_spec.rb

@@ -19,8 +19,8 @@
     end
 
     it 'should restrict only by searchstring' do
-      article = FactoryGirl.create(:article, :body => 'once uppon an originally time')
-      get :index, :search => {:searchstring => 'originally'}
+      article = create(:article, body: 'once uppon an originally time')
+      get :index, search: {searchstring: 'originally'}
       assigns(:articles).should == [article]
       response.should render_template('index')
       response.should be_success