Merge branch 'rs-dev-issue-2372' into 'master'

Filter by issues/merge requests without a milestone

Closes internal https://dev.gitlab.org/gitlab/gitlabhq/issues/2372

See merge request !886

Gemfile

@@ -203,7 +203,7 @@ gem 'jquery-ui-rails'
 gem 'nprogress-rails'
 gem 'raphael-rails',      '~> 2.1.2'
 gem 'request_store'
-gem 'select2-rails'
+gem 'select2-rails',      '~> 3.5.9'
 gem 'virtus'
 
 group :development do
@@ -250,7 +250,7 @@ group :development, :test do
   # Generate Fake data
   gem 'ffaker', '~> 2.0.0'
 
-  gem 'capybara',            '~> 2.3.0'
+  gem 'capybara',            '~> 2.4.0'
   gem 'capybara-screenshot', '~> 1.0.0'
   gem 'poltergeist',         '~> 1.6.0'
 

Gemfile.lock

@@ -82,7 +82,7 @@ GEM
       columnize (~> 0.8)
       debugger-linecache (~> 1.2)
     cal-heatmap-rails (0.0.1)
-    capybara (2.3.0)
+    capybara (2.4.4)
       mime-types (>= 1.16)
       nokogiri (>= 1.3.3)
       rack (>= 1.0.0)
@@ -598,7 +598,7 @@ GEM
     seed-fu (2.3.5)
       activerecord (>= 3.1, < 4.3)
       activesupport (>= 3.1, < 4.3)
-    select2-rails (3.5.2)
+    select2-rails (3.5.9.3)
       thor (~> 0.14)
     settingslogic (2.0.9)
     sexp_processor (4.4.5)
@@ -753,7 +753,7 @@ DEPENDENCIES
   browser (~> 0.8.0)
   byebug
   cal-heatmap-rails (~> 0.0.1)
-  capybara (~> 2.3.0)
+  capybara (~> 2.4.0)
   capybara-screenshot (~> 1.0.0)
   carrierwave
   charlock_holmes
@@ -842,7 +842,7 @@ DEPENDENCIES
   sass-rails (~> 4.0.5)
   sdoc
   seed-fu
-  select2-rails
+  select2-rails (~> 3.5.9)
   settingslogic
   shoulda-matchers (~> 2.8.0)
   sidekiq (~> 3.3)
@@ -878,4 +878,4 @@ DEPENDENCIES
   wikicloth (= 0.8.1)
 
 BUNDLED WITH
-   1.10.4
+   1.10.5

app/finders/issuable_finder.rb

@@ -10,7 +10,7 @@
 #     state: 'open' or 'closed' or 'all'
 #     group_id: integer
 #     project_id: integer
-#     milestone_id: integer
+#     milestone_title: string
 #     assignee_id: integer
 #     search: string
 #     label_name: string
@@ -76,7 +76,7 @@ class IssuableFinder
     return @milestones if defined?(@milestones)
 
     @milestones =
-      if milestones? && params[:milestone_title] != NONE
+      if milestones? && params[:milestone_title] != Milestone::None.title
         Milestone.where(title: params[:milestone_title])
       else
         nil

app/helpers/milestones_helper.rb

@@ -29,6 +29,8 @@ module MilestonesHelper
       end.active
 
     grouped_milestones = Milestones::GroupService.new(milestones).execute
+    grouped_milestones.unshift(Milestone::None)
+
     options_from_collection_for_select(grouped_milestones, 'title', 'title', params[:milestone_title])
   end
 end

app/models/milestone.rb

@@ -14,6 +14,10 @@
 #
 
 class Milestone < ActiveRecord::Base
+  # Represents a "No Milestone" state used for filtering Issues and Merge
+  # Requests that have no milestone assigned.
+  None = Struct.new(:title).new('No Milestone')
+
   include InternalId
   include Sortable
 

app/views/shared/issuable/_filter.html.haml

@@ -43,11 +43,15 @@
             placeholder: 'Author', class: 'trigger-submit', any_user: true, first_user: true)
 
         .filter-item.inline.milestone-filter
-          = select_tag('milestone_title', projects_milestones_options, class: "select2 trigger-submit", prompt: 'Milestone')
+          = select_tag('milestone_title', projects_milestones_options,
+            class: 'select2 trigger-submit', include_blank: true,
+            data: {placeholder: 'Milestone'})
 
         - if @project
           .filter-item.inline.labels-filter
-            = select_tag('label_name', project_labels_options(@project), class: "select2 trigger-submit", prompt: 'Label')
+            = select_tag('label_name', project_labels_options(@project),
+              class: 'select2 trigger-submit', include_blank: true,
+              data: {placeholder: 'Label'})
 
         .pull-right
           = render 'shared/sort_dropdown'

spec/features/groups_spec.rb

 require 'spec_helper'
 
-feature 'Group' do
+feature 'Group', feature: true do
   describe 'description' do
     let(:group) { create(:group) }
     let(:path)  { group_path(group) }

spec/features/issues/filter_by_milestone_spec.rb

+require 'rails_helper'
+
+feature 'Issue filtering by Milestone', feature: true do
+  include Select2Helper
+
+  let(:project)   { create(:project, :public) }
+  let(:milestone) { create(:milestone, project: project) }
+
+  scenario 'filters by no Milestone', js: true do
+    create(:issue, project: project)
+    create(:issue, project: project, milestone: milestone)
+
+    visit_issues(project)
+    filter_by_milestone(Milestone::None.title)
+
+    expect(page).to have_css('.issue-title', count: 1)
+  end
+
+  scenario 'filters by a specific Milestone', js: true do
+    create(:issue, project: project, milestone: milestone)
+    create(:issue, project: project)
+
+    visit_issues(project)
+    filter_by_milestone(milestone.title)
+
+    expect(page).to have_css('.issue-title', count: 1)
+  end
+
+  def visit_issues(project)
+    visit namespace_project_issues_path(project.namespace, project)
+  end
+
+  def filter_by_milestone(title)
+    select2(title, from: '#milestone_title')
+  end
+end

spec/features/issues_spec.rb

@@ -92,22 +92,6 @@ describe 'Issues', feature: true do
 
     let(:issue) { @issue }
 
-    it 'should allow filtering by issues with no specified milestone' do
-      visit namespace_project_issues_path(project.namespace, project, milestone_title: IssuableFinder::NONE)
-
-      expect(page).not_to have_content 'foobar'
-      expect(page).to have_content 'barbaz'
-      expect(page).to have_content 'gitlab'
-    end
-
-    it 'should allow filtering by a specified milestone' do
-      visit namespace_project_issues_path(project.namespace, project, milestone_title: issue.milestone.title)
-
-      expect(page).to have_content 'foobar'
-      expect(page).not_to have_content 'barbaz'
-      expect(page).not_to have_content 'gitlab'
-    end
-
     it 'should allow filtering by issues with no specified assignee' do
       visit namespace_project_issues_path(project.namespace, project, assignee_id: IssuableFinder::NONE)
 

spec/features/login_spec.rb

 require 'spec_helper'
 
-feature 'Login' do
+feature 'Login', feature: true do
   describe 'with two-factor authentication' do
     context 'with valid username/password' do
       let(:user) { create(:user, :two_factor) }

spec/features/markdown_spec.rb

@@ -32,7 +32,7 @@ require 'erb'
 #
 # See the MarkdownFeature class for setup details.
 
-describe 'GitLab Markdown' do
+describe 'GitLab Markdown', feature: true do
   include ActionView::Helpers::TagHelper
   include ActionView::Helpers::UrlHelper
   include Capybara::Node::Matchers

spec/features/merge_requests/filter_by_milestone_spec.rb

+require 'rails_helper'
+
+feature 'Merge Request filtering by Milestone', feature: true do
+  include Select2Helper
+
+  let(:project)   { create(:project, :public) }
+  let(:milestone) { create(:milestone, project: project) }
+
+  scenario 'filters by no Milestone', js: true do
+    create(:merge_request, :with_diffs, source_project: project)
+    create(:merge_request, :simple, source_project: project, milestone: milestone)
+
+    visit_merge_requests(project)
+    filter_by_milestone(Milestone::None.title)
+
+    expect(page).to have_css('.merge-request-title', count: 1)
+  end
+
+  scenario 'filters by a specific Milestone', js: true do
+    create(:merge_request, :with_diffs, source_project: project, milestone: milestone)
+    create(:merge_request, :simple, source_project: project)
+
+    visit_merge_requests(project)
+    filter_by_milestone(milestone.title)
+
+    expect(page).to have_css('.merge-request-title', count: 1)
+  end
+
+  def visit_merge_requests(project)
+    visit namespace_project_merge_requests_path(project.namespace, project)
+  end
+
+  def filter_by_milestone(title)
+    select2(title, from: '#milestone_title')
+  end
+end

spec/features/notes_on_merge_requests_spec.rb

 require 'spec_helper'
 
-describe 'Comments' do
+describe 'Comments', feature: true do
   include RepoHelpers
 
   describe 'On a merge request', js: true, feature: true do

spec/features/password_reset_spec.rb

 require 'spec_helper'
 
-feature 'Password reset' do
+feature 'Password reset', feature: true do
   def forgot_password
     click_on 'Forgot your password?'
     fill_in 'Email', with: user.email

spec/features/profile_spec.rb

@@ -9,8 +9,7 @@ describe 'Profile account page', feature: true do
 
   describe 'when signup is enabled' do
     before do
-      allow_any_instance_of(ApplicationSetting).
-        to receive(:signup_enabled?).and_return(true)
+      stub_application_setting(signup_enabled: true)
       visit profile_account_path
     end
 
@@ -24,8 +23,7 @@ describe 'Profile account page', feature: true do
 
   describe 'when signup is disabled' do
     before do
-      allow_any_instance_of(ApplicationSetting).
-        to receive(:signup_enabled?).and_return(false)
+      stub_application_setting(signup_enabled: false)
       visit profile_account_path
     end
 

spec/features/profiles/preferences_spec.rb

 require 'spec_helper'
 
-describe 'Profile > Preferences' do
+describe 'Profile > Preferences', feature: true do
   let(:user) { create(:user) }
 
   before do

spec/features/projects_spec.rb

 require 'spec_helper'
 
-feature 'Project' do
+feature 'Project', feature: true do
   describe 'description' do
     let(:project) { create(:project) }
     let(:path)    { namespace_project_path(project.namespace, project) }

spec/features/users_spec.rb

 require 'spec_helper'
 
-feature 'Users' do
+feature 'Users', feature: true do
   scenario 'GET /users/sign_in creates a new user account' do
     visit new_user_session_path
     fill_in 'user_name', with: 'Name Surname'

spec/requests/api/projects_spec.rb

@@ -220,9 +220,7 @@ describe API::API, api: true  do
     context 'when a visibility level is restricted' do
       before do
         @project = attributes_for(:project, { public: true })
-        allow_any_instance_of(ApplicationSetting).to(
-          receive(:restricted_visibility_levels).and_return([20])
-        )
+        stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
       end
 
       it 'should not allow a non-admin to use a restricted visibility level' do

spec/services/create_snippet_service_spec.rb

@@ -14,11 +14,7 @@ describe CreateSnippetService do
 
   context 'When public visibility is restricted' do
     before do
-      allow_any_instance_of(ApplicationSetting).to(
-        receive(:restricted_visibility_levels).and_return(
-          [Gitlab::VisibilityLevel::PUBLIC]
-        )
-      )
+      stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
 
       @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
     end

spec/services/git_push_service_spec.rb

@@ -124,9 +124,7 @@ describe GitPushService do
       end
 
       it "when pushing a branch for the first time with default branch protection disabled" do
-        allow(ApplicationSetting.current_application_settings).
-          to receive(:default_branch_protection).
-          and_return(Gitlab::Access::PROTECTION_NONE)
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE)
 
         expect(project).to receive(:execute_hooks)
         expect(project.default_branch).to eq("master")
@@ -135,9 +133,7 @@ describe GitPushService do
       end
 
       it "when pushing a branch for the first time with default branch protection set to 'developers can push'" do
-        allow(ApplicationSetting.current_application_settings).
-          to receive(:default_branch_protection).
-          and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH)
+        stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH)
 
         expect(project).to receive(:execute_hooks)
         expect(project.default_branch).to eq("master")

spec/services/projects/create_service_spec.rb

@@ -58,9 +58,7 @@ describe Projects::CreateService do
 
     context 'restricted visibility level' do
       before do
-        allow_any_instance_of(ApplicationSetting).to(
-          receive(:restricted_visibility_levels).and_return([20])
-        )
+        stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
 
         @opts.merge!(
           visibility_level: Gitlab::VisibilityLevel.options['Public']

spec/services/projects/update_service_spec.rb

@@ -47,9 +47,7 @@ describe Projects::UpdateService do
 
     context 'respect configured visibility restrictions setting' do
       before(:each) do
-        allow_any_instance_of(ApplicationSetting).to(
-          receive(:restricted_visibility_levels).and_return([20])
-        )
+        stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
       end
 
       context 'should be private when updated to private' do

spec/services/update_snippet_service_spec.rb

@@ -14,11 +14,7 @@ describe UpdateSnippetService do
 
   context 'When public visibility is restricted' do
     before do
-      allow_any_instance_of(ApplicationSetting).to(
-        receive(:restricted_visibility_levels).and_return(
-          [Gitlab::VisibilityLevel::PUBLIC]
-        )
-      )
+      stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
 
       @snippet = create_snippet(@project, @user, @opts)
       @opts.merge!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)

spec/support/stub_configuration.rb

 module StubConfiguration
   def stub_application_setting(messages)
+    add_predicates(messages)
+
+    # Stubbing both of these because we're not yet consistent with how we access
+    # current application settings
+    allow_any_instance_of(ApplicationSetting).to receive_messages(messages)
     allow(Gitlab::CurrentSettings.current_application_settings).
       to receive_messages(messages)
   end
@@ -11,4 +16,25 @@ module StubConfiguration
   def stub_gravatar_setting(messages)
     allow(Gitlab.config.gravatar).to receive_messages(messages)
   end
+
+  private
+
+  # Modifies stubbed messages to also stub possible predicate versions
+  #
+  # Examples:
+  #
+  #   add_predicates(foo: true)
+  #   # => {foo: true, foo?: true}
+  #
+  #   add_predicates(signup_enabled?: false)
+  #   # => {signup_enabled? false}
+  def add_predicates(messages)
+    # Only modify keys that aren't already predicates
+    keys = messages.keys.map(&:to_s).reject { |k| k.end_with?('?') }
+
+    keys.each do |key|
+      predicate = key + '?'
+      messages[predicate.to_sym] = messages[key.to_sym]
+    end
+  end
 end