diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb
index bd644e572..cdca0a1e3 100644
--- a/app/controllers/list_controller.rb
+++ b/app/controllers/list_controller.rb
@@ -14,16 +14,11 @@ class ListController < ApplicationController
list_opts[:topic_ids] = params[:topic_ids].split(",").map(&:to_i)
end
- # html format means we need to farm exclude from the site options
- if params[:format].blank? || params[:format] == "html"
- #TODO objectify this stuff
- SiteSetting.top_menu.split('|').each do |f|
- s = f.split(",")
- if s[0] == action_name || (action_name == "index" && s[0] == SiteSetting.homepage)
- list_opts[:exclude_category] = s[1][1..-1] if s.length == 2
- end
- end
- end
+ # html format means we need to parse exclude from the site options top menu
+ menu_item = SiteSetting.top_menu_items.select { |menu_item|
+ menu_item.query_should_exclude_category?(action_name, params[:format])
+ }.first
+ list_opts[:exclude_category] = menu_item.try(:filter)
list_opts[:exclude_category] = params[:exclude_category] if params[:exclude_category].present?
list = TopicQuery.new(current_user, list_opts).send("list_#{filter}")
diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb
index afe26693f..01be118cb 100644
--- a/app/models/site_setting.rb
+++ b/app/models/site_setting.rb
@@ -253,14 +253,17 @@ class SiteSetting < ActiveRecord::Base
min_private_message_post_length..max_post_length
end
+ def self.top_menu_items
+ top_menu.split('|').map { |menu_item| TopMenuItem.new(menu_item) }
+ end
+
def self.homepage
- # TODO objectify this
- top_menu.split('|')[0].split(',')[0]
+ top_menu_items[0].name
end
def self.anonymous_homepage
- # TODO objectify this
- top_menu.split('|').map{|f| f.split(',')[0] }.select{ |f| ['latest', 'hot', 'categories', 'category'].include? f}[0]
+ list = ['latest', 'hot', 'categories', 'category']
+ top_menu_items.map { |item| item.name }.select{ |item| list.include?(item) }.first
end
end
diff --git a/app/models/top_menu_item.rb b/app/models/top_menu_item.rb
new file mode 100644
index 000000000..9f7a22b54
--- /dev/null
+++ b/app/models/top_menu_item.rb
@@ -0,0 +1,72 @@
+# Public: Instances of TopMenuItem should be instantiated from segments contained in SiteSetting.top_menu.
+# Exposes relevant properties and methods that dictate which query methods should be called from the ListController.
+# Segment data should start with a route fragment in one of the following formats:
+# a topic route, such as 'latest' or 'new' (see ListController for valid options)
+# the literal string "categories"
+# a specific category route, must start with 'category/' followed by the route, i.e. 'category/xyz'
+#
+# A topic route can optionally specify a single category to exclude using the '-category' option, i.e. 'new,-xyz'
+#
+# Examples
+#
+# item = TopMenuItem.new('unread')
+# item.name # => "unread"
+#
+# item = TopMenuItem.new('latest,-video')
+# item.name # => "latest"
+# item.has_filter? # => true
+# item.filter # => "video"
+#
+# item = TopMenuItem.new('category/hardware')
+# item.name # => "category"
+# item.has_filter? # => false
+# item.has_specific_category? # => true
+# item.specific_category # => "hardware"
+class TopMenuItem
+ def initialize(value)
+ parts = value.split(',')
+ @name = parts[0]
+ @filter = initialize_filter(parts[1])
+ end
+
+ attr_reader :name, :filter
+
+ def has_filter?
+ !filter.nil?
+ end
+
+ def has_specific_category?
+ name.split('/')[0] == 'category'
+ end
+
+ def specific_category
+ name.split('/')[1]
+ end
+
+ def query_should_exclude_category?(action_name, format)
+ if format.blank? || format == "html"
+ matches_action?(action_name) && has_filter?
+ else
+ false
+ end
+ end
+
+ def matches_action?(action_name)
+ return true if action_name == "index" && name == SiteSetting.homepage
+ return true if name == action_name
+ false
+ end
+
+ private
+
+ def initialize_filter(value)
+ if value
+ if value.start_with?('-')
+ value[1..-1] # all but the leading -
+ else
+ Rails.logger.warn "WARNING: found top_menu_item with invalid filter, ignoring '#{value}'..."
+ nil
+ end
+ end
+ end
+end
\ No newline at end of file
diff --git a/spec/controllers/list_controller_spec.rb b/spec/controllers/list_controller_spec.rb
index 660d18863..bbd6fd355 100644
--- a/spec/controllers/list_controller_spec.rb
+++ b/spec/controllers/list_controller_spec.rb
@@ -6,19 +6,22 @@ describe ListController do
before do
@user = Fabricate(:coding_horror)
@post = Fabricate(:post, user: @user)
+
+ # forces tests down some code paths
+ SiteSetting.stubs(:top_menu).returns('latest,-video|new|unread|favorited|categories|category/beer')
end
describe 'indexes' do
[:latest, :hot].each do |filter|
- context '#{filter}' do
+ context "#{filter}" do
before { xhr :get, filter }
it { should respond_with(:success) }
end
end
[:favorited, :read, :posted, :unread, :new].each do |filter|
- context '#{filter}' do
+ context "#{filter}" do
it { expect { xhr :get, filter }.to raise_error(Discourse::NotLoggedIn) }
end
end
diff --git a/spec/models/site_setting_spec.rb b/spec/models/site_setting_spec.rb
index 103eaac56..5fba2abd9 100644
--- a/spec/models/site_setting_spec.rb
+++ b/spec/models/site_setting_spec.rb
@@ -63,7 +63,30 @@ describe SiteSetting do
it 'is always the correct default' do
expect(SiteSetting.contact_email).to eq('')
end
+ end
+ describe "anonymous_homepage" do
+ it "returns latest" do
+ expect(SiteSetting.anonymous_homepage).to eq('latest')
+ end
+ end
+
+ describe "top_menu" do
+ before(:each) { SiteSetting.stubs(:top_menu).returns('one,-nope|two|three,-not|four,ignored|category/xyz') }
+
+ describe "items" do
+ let(:items) { SiteSetting.top_menu_items }
+
+ it 'returns TopMenuItem objects' do
+ expect(items[0]).to be_kind_of(TopMenuItem)
+ end
+ end
+
+ describe "homepage" do
+ it "has homepage" do
+ expect(SiteSetting.homepage).to eq('one')
+ end
+ end
end
end
diff --git a/spec/models/top_menu_item_spec.rb b/spec/models/top_menu_item_spec.rb
new file mode 100644
index 000000000..7e2c12edb
--- /dev/null
+++ b/spec/models/top_menu_item_spec.rb
@@ -0,0 +1,69 @@
+require 'spec_helper'
+
+describe TopMenuItem do
+ before(:each) { SiteSetting.stubs(:top_menu).returns('one,-nope|two|three,-not|four,ignored|category/xyz') }
+ let(:items) { SiteSetting.top_menu_items }
+
+ it 'has name' do
+ expect(items[0].name).to eq('one')
+ expect(items[1].name).to eq('two')
+ expect(items[2].name).to eq('three')
+ end
+
+ it 'has a filter' do
+ expect(items[0].filter).to eq('nope')
+ expect(items[0].has_filter?).to be_true
+ expect(items[2].filter).to eq('not')
+ expect(items[2].has_filter?).to be_true
+ end
+
+ it 'does not have a filter' do
+ expect(items[1].filter).to be_nil
+ expect(items[1].has_filter?).to be_false
+ expect(items[3].filter).to be_nil
+ expect(items[3].has_filter?).to be_false
+ end
+
+ it "has a specific category" do
+ expect(items.first.has_specific_category?).to be_false
+ expect(items.last.has_specific_category?).to be_true
+ end
+
+ it "does not have a specific category" do
+ expect(items.first.specific_category).to be_nil
+ expect(items.last.specific_category).to eq('xyz')
+ end
+
+ describe "matches_action?" do
+ it "does not match index on other pages" do
+ expect(TopMenuItem.new('xxxx').matches_action?("index")).to be_false
+ end
+
+ it "matches index on homepage" do
+ expect(items[0].matches_action?("index")).to be_true
+ end
+
+ it "matches current action" do
+ expect(items[1].matches_action?("two")).to be_true
+ end
+
+ it "does not match current action" do
+ expect(items[1].matches_action?("one")).to be_false
+ end
+ end
+
+ describe "query_should_exclude_category" do
+ before(:each) do
+ items[0].stubs(:matches_action?).returns(true)
+ items[0].stubs(:has_filter?).returns(true)
+ end
+
+ it "excludes category" do
+ expect(items[0].query_should_exclude_category?(nil, nil)).to be_true
+ end
+
+ it "does not exclude for json format" do
+ expect(items[0].query_should_exclude_category?(nil, 'json')).to be_false
+ end
+ end
+end
\ No newline at end of file