diff --git a/app/controllers/list_controller.rb b/app/controllers/list_controller.rb index bd644e572bb..cdca0a1e350 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 afe26693f55..01be118cb9f 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 00000000000..9f7a22b54b5 --- /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 660d18863ba..bbd6fd355aa 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 103eaac56ae..5fba2abd994 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 00000000000..7e2c12edb40 --- /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