From 3281021e8b42ba4d0fff5be268f5c43dd6ab4ff8 Mon Sep 17 00:00:00 2001
From: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Date: Sat, 10 Aug 2013 20:25:53 +0300
Subject: [PATCH] Rewrite issues/mr filtering logic

---
 app/contexts/issues/list_context.rb           | 20 ++++---
 app/contexts/merge_requests_load_context.rb   | 14 +++--
 app/controllers/projects/issues_controller.rb |  2 -
 .../projects/merge_requests_controller.rb     |  3 ++
 app/helpers/issues_helper.rb                  | 32 +----------
 app/helpers/projects_helper.rb                | 19 +++++++
 app/models/concerns/issuable.rb               |  1 +
 app/models/issue.rb                           |  1 -
 app/views/projects/issues/_filter.html.haml   | 29 +++++-----
 app/views/projects/issues/_issues.html.haml   | 20 +++----
 app/views/projects/issues/index.html.haml     |  2 +-
 .../projects/merge_requests/index.html.haml   | 54 ++++++++++++++++---
 app/views/shared/_filter.html.haml            |  2 +-
 app/views/shared/_project_filter.html.haml    | 31 +++++++++++
 14 files changed, 153 insertions(+), 77 deletions(-)
 create mode 100644 app/views/shared/_project_filter.html.haml

diff --git a/app/contexts/issues/list_context.rb b/app/contexts/issues/list_context.rb
index 906a83b58a6..da2eed0e259 100644
--- a/app/contexts/issues/list_context.rb
+++ b/app/contexts/issues/list_context.rb
@@ -1,16 +1,20 @@
 module Issues
   class ListContext < BaseContext
-    include IssuesHelper
-
     attr_accessor :issues
 
     def execute
-      @issues = case params[:status]
-                when issues_filter[:all] then @project.issues
-                when issues_filter[:closed] then @project.issues.closed
-                when issues_filter[:to_me] then @project.issues.assigned_to(current_user)
-                when issues_filter[:by_me] then @project.issues.authored(current_user)
-                else @project.issues.opened
+      @issues = @project.issues
+
+      @issues = case params[:state]
+                when 'all' then @issues
+                when 'closed' then @issues.closed
+                else @issues.opened
+                end
+
+      @issues = case params[:scope]
+                when 'assigned-to-me' then @issues.assigned_to(current_user)
+                when 'created-by-me' then @issues.authored(current_user)
+                else @issues
                 end
 
       @issues = @issues.tagged_with(params[:label_name]) if params[:label_name].present?
diff --git a/app/contexts/merge_requests_load_context.rb b/app/contexts/merge_requests_load_context.rb
index 40b4c7bac74..c3408db6e11 100644
--- a/app/contexts/merge_requests_load_context.rb
+++ b/app/contexts/merge_requests_load_context.rb
@@ -2,17 +2,21 @@
 # based on filtering passed via params for @project
 class MergeRequestsLoadContext < BaseContext
   def execute
-    type = params[:status]
+    merge_requests = @project.merge_requests
 
-    merge_requests = project.merge_requests
-
-    merge_requests = case type
+    merge_requests = case params[:state]
                      when 'all' then merge_requests
                      when 'closed' then merge_requests.closed
-                     when 'assigned-to-me' then merge_requests.opened.assigned_to(current_user)
                      else merge_requests.opened
                      end
 
+    merge_requests = case params[:scope]
+                     when 'assigned-to-me' then merge_requests.assigned_to(current_user)
+                     when 'created-by-me' then merge_requests.authored(current_user)
+                     else merge_requests
+                     end
+
+
     merge_requests = merge_requests.page(params[:page]).per(20)
     merge_requests = merge_requests.includes(:author, :source_project, :target_project).order("created_at desc")
 
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 06f8a1233a3..a7f515ac851 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -20,9 +20,7 @@ class Projects::IssuesController < Projects::ApplicationController
     @issues = @issues.where("title LIKE ?", "%#{terms}%") if terms.present?
     @issues = @issues.page(params[:page]).per(20)
 
-
     assignee_id, milestone_id = params[:assignee_id], params[:milestone_id]
-
     @assignee = @project.team.find(assignee_id) if assignee_id.present? && !assignee_id.to_i.zero?
     @milestone = @project.milestones.find(milestone_id) if milestone_id.present? && !milestone_id.to_i.zero?
 
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index aa244143438..d135cf05b9e 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -17,6 +17,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   def index
     @merge_requests = MergeRequestsLoadContext.new(project, current_user, params).execute
+    assignee_id, milestone_id = params[:assignee_id], params[:milestone_id]
+    @assignee = @project.team.find(assignee_id) if assignee_id.present? && !assignee_id.to_i.zero?
+    @milestone = @project.milestones.find(milestone_id) if milestone_id.present? && !milestone_id.to_i.zero?
   end
 
   def show
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index d7d50db8a2f..4f433f3c60d 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -1,9 +1,4 @@
 module IssuesHelper
-  def project_issues_filter_path project, params = {}
-    params[:f] ||= cookies['issue_filter']
-    project_issues_path project, params
-  end
-
   def issue_css_classes issue
     classes = "issue"
     classes << " closed" if issue.closed?
@@ -18,25 +13,11 @@ module IssuesHelper
     OpenStruct.new(id: 0, title: 'None (backlog)', name: 'Unassigned')
   end
 
-  def issues_filter
-    {
-      all: "all",
-      closed: "closed",
-      to_me: "assigned-to-me",
-      by_me: "created-by-me",
-      open: "open"
-    }
-  end
-
-  def issues_active_milestones
-    @project.milestones.active.order("id desc").all
-  end
-
   def url_for_project_issues
     return "" if @project.nil?
 
     if @project.used_default_issues_tracker?
-      project_issues_filter_path(@project)
+      project_issues_path(@project)
     else
       url = Gitlab.config.issues_tracker[@project.issues_tracker]["project_url"]
       url.gsub(':project_id', @project.id.to_s)
@@ -78,15 +59,4 @@ module IssuesHelper
       ""
     end
   end
-
-  def project_issues_with_filter_path(project, opts)
-    default_opts = {
-      status: params[:status],
-      label_name: params[:label_name],
-      milestone_id: params[:milestone_id],
-      assignee_id: params[:assignee_id],
-    }
-
-    project_issues_path(@project, default_opts.merge(opts))
-  end
 end
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index f742b00bb7f..3a1cf59fd1a 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -61,6 +61,25 @@ module ProjectsHelper
     project_nav_tabs.include? name
   end
 
+  def project_filter_path(options={})
+    exist_opts = {
+      state: params[:state],
+      scope: params[:scope],
+      label_name: params[:label_name],
+      milestone_id: params[:milestone_id],
+    }
+
+    options = exist_opts.merge(options)
+
+    path = request.path
+    path << "?#{options.to_param}"
+    path
+  end
+
+  def project_active_milestones
+    @project.milestones.active.order("id desc").all
+  end
+
   private
 
   def get_project_nav_tabs(project, current_user)
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 24818a927ed..91fb323825d 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -17,6 +17,7 @@ module Issuable
     validates :author, presence: true
     validates :title, presence: true, length: { within: 0..255 }
 
+    scope :authored, ->(user) { where(author_id: user) }
     scope :assigned_to, ->(u) { where(assignee_id: u.id)}
     scope :recent, -> { order("created_at DESC") }
     scope :assigned, -> { where("assignee_id IS NOT NULL") }
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 550041a0224..ecb881ab597 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -35,7 +35,6 @@ class Issue < ActiveRecord::Base
   acts_as_taggable_on :labels
 
   scope :cared, ->(user) { where(assignee_id: user) }
-  scope :authored, ->(user) { where(author_id: user) }
   scope :open_for, ->(user) { opened.assigned_to(user) }
 
   state_machine :state, initial: :opened do
diff --git a/app/views/projects/issues/_filter.html.haml b/app/views/projects/issues/_filter.html.haml
index 663d786634d..971096427ea 100644
--- a/app/views/projects/issues/_filter.html.haml
+++ b/app/views/projects/issues/_filter.html.haml
@@ -1,24 +1,29 @@
 = form_tag project_issues_path(@project), method: 'get' do
   %fieldset
     %ul.nav.nav-pills.nav-stacked
-      %li{class: ("active" if !params[:status] || params[:status].blank?)}
-        = link_to project_issues_path(@project, status: nil) do
-          Open
-      %li{class: ("active" if params[:status] == 'assigned-to-me')}
-        = link_to project_issues_path(@project, status: 'assigned-to-me') do
+      %li{class: ("active" if params[:scope].blank?)}
+        = link_to project_filter_path(scope: nil) do
+          Everyone's
+      %li{class: ("active" if params[:scope] == 'assigned-to-me')}
+        = link_to project_filter_path(scope: 'assigned-to-me') do
           Assigned to me
-      %li{class: ("active" if params[:status] == 'created-by-me')}
-        = link_to project_issues_path(@project, status: 'created-by-me') do
+      %li{class: ("active" if params[:scope] == 'created-by-me')}
+        = link_to project_filter_path(scope: 'created-by-me') do
           Created by me
-      %li{class: ("active" if params[:status] == 'closed')}
-        = link_to project_issues_path(@project, status: 'closed') do
+
+    %ul.nav.nav-pills.nav-stacked
+      %li{class: ("active" if params[:state].blank?)}
+        = link_to project_filter_path(state: nil) do
+          Open
+      %li{class: ("active" if params[:state] == 'closed')}
+        = link_to project_filter_path(state: 'closed') do
           Closed
-      %li{class: ("active" if params[:status] == 'all')}
-        = link_to project_issues_path(@project, status: 'all') do
+      %li{class: ("active" if params[:state] == 'all')}
+        = link_to project_filter_path(state: 'all') do
           All
 
   %fieldset
-    - if %w(status milestone_id assignee_id label_name).select { |k| params[k].present? }.any?
+    - if %w(state scope milestone_id assignee_id label_name).select { |k| params[k].present? }.any?
       = link_to project_issues_path(@project), class: 'cgray pull-right' do
         %i.icon-remove
         Clear filter
diff --git a/app/views/projects/issues/_issues.html.haml b/app/views/projects/issues/_issues.html.haml
index 231574be72c..99a23c169d2 100644
--- a/app/views/projects/issues/_issues.html.haml
+++ b/app/views/projects/issues/_issues.html.haml
@@ -7,7 +7,7 @@
           %span Update selected issues with &nbsp;
           = select_tag('update[status]', options_for_select(['open', 'closed']), prompt: "Status")
           = select_tag('update[assignee_id]', options_from_collection_for_select(@project.team.members, "id", "name", params[:assignee_id]), prompt: "Assignee")
-          = select_tag('update[milestone_id]', options_from_collection_for_select(issues_active_milestones, "id", "title", params[:milestone_id]), prompt: "Milestone")
+          = select_tag('update[milestone_id]', options_from_collection_for_select(project_active_milestones, "id", "title", params[:milestone_id]), prompt: "Milestone")
           = hidden_field_tag 'update[issues_ids]', []
           = hidden_field_tag :status, params[:status]
           = button_tag "Save", class: "btn update_selected_issues btn-small btn-save"
@@ -24,11 +24,11 @@
             %b.caret
           %ul.dropdown-menu
             %li
-              = link_to project_issues_with_filter_path(@project, label_name: nil) do
+              = link_to project_filter_path(label_name: nil) do
                 Any
             - issue_label_names.each do |label_name|
               %li
-                = link_to project_issues_with_filter_path(@project, label_name: label_name) do
+                = link_to project_filter_path(label_name: label_name) do
                   %span{class: "label #{label_css_class(label_name)}"}
                     %i.icon-tag
                   = label_name
@@ -45,13 +45,13 @@
             %b.caret
           %ul.dropdown-menu
             %li
-              = link_to project_issues_with_filter_path(@project, assignee_id: nil) do
+              = link_to project_filter_path(assignee_id: nil) do
                 Any
-              = link_to project_issues_with_filter_path(@project, assignee_id: 0) do
+              = link_to project_filter_path(assignee_id: 0) do
                 Unassigned
             - @project.team.members.sort_by(&:name).each do |user|
               %li
-                = link_to project_issues_with_filter_path(@project, assignee_id: user.id) do
+                = link_to project_filter_path(assignee_id: user.id) do
                   = image_tag gravatar_icon(user.email), class: "avatar s16", alt: ''
                   = user.name
 
@@ -68,13 +68,13 @@
             %b.caret
           %ul.dropdown-menu
             %li
-              = link_to project_issues_with_filter_path(@project, milestone_id: nil) do
+              = link_to project_filter_path(milestone_id: nil) do
                 Any
-              = link_to project_issues_with_filter_path(@project, milestone_id: 0) do
+              = link_to project_filter_path(milestone_id: 0) do
                 None (backlog)
-            - issues_active_milestones.each do |milestone|
+            - project_active_milestones.each do |milestone|
               %li
-                = link_to project_issues_with_filter_path(@project, milestone_id: milestone.id) do
+                = link_to project_filter_path(milestone_id: milestone.id) do
                   %strong= milestone.title
                   %small.light= milestone.expires_at
 
diff --git a/app/views/projects/issues/index.html.haml b/app/views/projects/issues/index.html.haml
index 50df3ee3b7a..81594b4972e 100644
--- a/app/views/projects/issues/index.html.haml
+++ b/app/views/projects/issues/index.html.haml
@@ -18,6 +18,6 @@
 
 .row
   .span3
-    = render 'filter', entity: 'issue'
+    = render 'shared/project_filter', project_entities_path: project_issues_path(@project)
   .span9.issues-holder
     = render "issues"
diff --git a/app/views/projects/merge_requests/index.html.haml b/app/views/projects/merge_requests/index.html.haml
index 9b9da8d1b8f..7970745e4c8 100644
--- a/app/views/projects/merge_requests/index.html.haml
+++ b/app/views/projects/merge_requests/index.html.haml
@@ -8,15 +8,57 @@
 
 .row
   .span3
-    = render 'filter'
+    = render 'shared/project_filter', project_entities_path: project_merge_requests_path(@project)
   .span9
     .ui-box
       .title
-        = form_tag project_merge_requests_path(@project), id: "merge_requests_search_form", method: :get, class: :left  do
-          = select_tag(:assignee_id, options_from_collection_for_select([unassigned_filter] + @project.users.all, "id", "name", params[:assignee_id]), prompt: "Assignee")
-          = select_tag(:milestone_id, options_from_collection_for_select([unassigned_filter] + @project.milestones.order("id desc").all, "id", "title", params[:milestone_id]), prompt: "Milestone")
-          = hidden_field_tag :f, params[:f]
-        .clearfix
+        .mr-filters
+          %span Filter by
+          .dropdown.inline.prepend-left-10
+            %a.dropdown-toggle.btn.btn-small{href: '#', "data-toggle" => "dropdown"}
+              %i.icon-user
+              %span.light assignee:
+              - if @assignee.present?
+                %strong= @assignee.name
+              - elsif params[:assignee_id] == "0"
+                Unassigned
+              - else
+                Any
+              %b.caret
+            %ul.dropdown-menu
+              %li
+                = link_to project_filter_path(assignee_id: nil) do
+                  Any
+                = link_to project_filter_path(assignee_id: 0) do
+                  Unassigned
+              - @project.team.members.sort_by(&:name).each do |user|
+                %li
+                  = link_to project_filter_path(assignee_id: user.id) do
+                    = image_tag gravatar_icon(user.email), class: "avatar s16", alt: ''
+                    = user.name
+
+          .dropdown.inline.prepend-left-10
+            %a.dropdown-toggle.btn.btn-small{href: '#', "data-toggle" => "dropdown"}
+              %i.icon-time
+              %span.light milestone:
+              - if @milestone.present?
+                %strong= @milestone.title
+              - elsif params[:milestone_id] == "0"
+                None (backlog)
+              - else
+                Any
+              %b.caret
+            %ul.dropdown-menu
+              %li
+                = link_to project_filter_path(milestone_id: nil) do
+                  Any
+                = link_to project_filter_path(milestone_id: 0) do
+                  None (backlog)
+              - project_active_milestones.each do |milestone|
+                %li
+                  = link_to project_filter_path(milestone_id: milestone.id) do
+                    %strong= milestone.title
+                    %small.light= milestone.expires_at
 
       %ul.well-list.mr-list
         = render @merge_requests
diff --git a/app/views/shared/_filter.html.haml b/app/views/shared/_filter.html.haml
index fc3232e0e0e..52ca1f22d7d 100644
--- a/app/views/shared/_filter.html.haml
+++ b/app/views/shared/_filter.html.haml
@@ -25,5 +25,5 @@
     - if params[:status].present? || params[:project_id].present?
       = link_to filter_path(entity, status: nil, project_id: nil), class: 'pull-right cgray' do
         %i.icon-remove
-        Clear filter
+        %strong Clear filter
 
diff --git a/app/views/shared/_project_filter.html.haml b/app/views/shared/_project_filter.html.haml
new file mode 100644
index 00000000000..5e0f503c819
--- /dev/null
+++ b/app/views/shared/_project_filter.html.haml
@@ -0,0 +1,31 @@
+= form_tag project_entities_path, method: 'get' do
+  %fieldset
+    %ul.nav.nav-pills.nav-stacked
+      %li{class: ("active" if params[:scope].blank?)}
+        = link_to project_filter_path(scope: nil) do
+          Everyone's
+      %li{class: ("active" if params[:scope] == 'assigned-to-me')}
+        = link_to project_filter_path(scope: 'assigned-to-me') do
+          Assigned to me
+      %li{class: ("active" if params[:scope] == 'created-by-me')}
+        = link_to project_filter_path(scope: 'created-by-me') do
+          Created by me
+
+    %ul.nav.nav-pills.nav-stacked
+      %li{class: ("active" if params[:state].blank?)}
+        = link_to project_filter_path(state: nil) do
+          Open
+      %li{class: ("active" if params[:state] == 'closed')}
+        = link_to project_filter_path(state: 'closed') do
+          Closed
+      %li{class: ("active" if params[:state] == 'all')}
+        = link_to project_filter_path(state: 'all') do
+          All
+
+  %fieldset
+    - if %w(state scope milestone_id assignee_id label_name).select { |k| params[k].present? }.any?
+      = link_to project_entities_path, class: 'cgray pull-right' do
+        %i.icon-remove
+        %strong Clear filter
+
+