Remove duplication of action defintion, add missing spec for topics_by, extract out URL construction logic

app/controllers/list_controller.rb

@@ -10,7 +10,7 @@ class ListController < ApplicationController
       list_opts = build_topic_list_options
       user = list_target_user
       list = TopicQuery.new(user, list_opts).public_send("list_#{filter}")
-      list.more_topics_url = url_for(self.public_send "#{filter}_path".to_sym, list_opts.merge(format: 'json', page: next_page))
+      list.more_topics_url = construct_url_with(filter, list_opts)
       if [:latest, :hot].include?(filter)
         @description = SiteSetting.site_description
         @rss = filter
@@ -33,45 +33,17 @@ class ListController < ApplicationController
     end
   end
 
-  def topics_by
-    list_opts = build_topic_list_options
-    list = TopicQuery.new(current_user, list_opts).list_topics_by(fetch_user_from_params)
-    list.more_topics_url = url_for(topics_by_path(list_opts.merge(format: 'json', page: next_page)))
-
-    respond(list)
-  end
-
-  def private_messages
-    list_opts = build_topic_list_options
-    target_user = fetch_user_from_params
-    guardian.ensure_can_see_private_messages!(target_user.id)
-
-    list = TopicQuery.new(current_user, list_opts).list_private_messages(target_user)
-    list.more_topics_url = url_for(topics_private_messages_path(list_opts.merge(format: 'json', page: next_page)))
-
-    respond(list)
-  end
-
-  def private_messages_sent
-    list_opts = build_topic_list_options
-    target_user = fetch_user_from_params
-    guardian.ensure_can_see_private_messages!(target_user.id)
-
-    list = TopicQuery.new(current_user, list_opts).list_private_messages_sent(target_user)
-    list.more_topics_url = url_for(topics_private_messages_sent_path(list_opts.merge(format: 'json', page: next_page)))
-
-    respond(list)
-  end
-
-  def private_messages_unread
-    list_opts = build_topic_list_options
-    target_user = fetch_user_from_params
-    guardian.ensure_can_see_private_messages!(target_user.id)
-
-    list = TopicQuery.new(current_user, list_opts).list_private_messages_unread(target_user)
-    list.more_topics_url = url_for(topics_private_messages_unread_path(list_opts.merge(format: 'json', page: next_page)))
-
-    respond(list)
+  [:topics_by, :private_messages, :private_messages_sent, :private_messages_unread].each do |action|
+    define_method("#{action}") do
+      list_opts = build_topic_list_options
+      target_user = fetch_user_from_params
+      guardian.ensure_can_see_private_messages!(target_user.id) unless action == :topics_by
+      list = generate_list_for(action.to_s, target_user, list_opts)
+      url_prefix = "topics" unless action == :topics_by
+      url  = construct_url_with(action, list_opts, url_prefix)
+      list.more_topics_url = url_for(url)
+      respond(list)
+    end
   end
 
   def category
@@ -98,7 +70,6 @@ class ListController < ApplicationController
     raise Discourse::InvalidParameters.new('Category RSS of "uncategorized"') if request_is_for_uncategorized?
 
     guardian.ensure_can_see!(@category)
-
     discourse_expires_in 1.minute
 
     @title = @category.name
@@ -176,4 +147,14 @@ class ListController < ApplicationController
       current_user
     end
   end
+
+  def generate_list_for(action, target_user, opts)
+    list = TopicQuery.new(current_user, opts)
+    list = list.send("list_#{action}", target_user)
+  end
+
+  def construct_url_with(action, opts, url_prefix=nil)
+    method = url_prefix.blank? ? "#{action}_path" : "#{url_prefix}_#{action}_path"
+    public_send(method, opts.merge(format: 'json', page: next_page))
+  end
 end

spec/controllers/list_controller_spec.rb

@@ -133,6 +133,15 @@ describe ListController do
 
   end
 
+  describe "topics_by" do
+    let!(:user) { log_in }
+
+    it "should respond with a list" do
+      xhr :get, :topics_by, username: @user.username
+      response.should be_success
+    end
+  end
+
   context "private_messages" do
     let!(:user) { log_in }