From 7a2370f74060b2f065e3602700fe1b33fda4685c Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Mon, 4 Apr 2016 21:25:38 -0400
Subject: Standardize the way we check for and display form errors

- Some views had a "Close" button. We've removed this, because we don't
  want users accidentally hiding the validation errors and not knowing
  what needs to be fixed.
- Some views used `li`, some used `p`, some used `span`. We've
  standardized on `li`.
- Some views only showed the first error. We've standardized on showing
  all of them.
- Some views added an `#error_explanation` div, which we've made
  standard.
---
 app/helpers/form_helper.rb                         | 18 +++++++++
 app/views/abuse_reports/new.html.haml              |  6 +--
 app/views/admin/appearances/_form.html.haml        |  5 +--
 .../admin/application_settings/_form.html.haml     |  6 +--
 app/views/admin/applications/_form.html.haml       |  7 +---
 app/views/admin/broadcast_messages/_form.html.haml |  6 +--
 app/views/admin/deploy_keys/new.html.haml          |  6 +--
 app/views/admin/groups/_form.html.haml             |  5 +--
 app/views/admin/hooks/index.html.haml              |  6 +--
 app/views/admin/identities/_form.html.haml         |  6 +--
 app/views/admin/labels/_form.html.haml             |  8 +---
 app/views/admin/users/_form.html.haml              |  6 +--
 app/views/doorkeeper/applications/_form.html.haml  |  6 +--
 app/views/groups/edit.html.haml                    |  4 +-
 app/views/groups/new.html.haml                     |  5 +--
 app/views/profiles/keys/_form.html.haml            |  6 +--
 app/views/profiles/notifications/show.html.haml    |  6 +--
 app/views/profiles/passwords/edit.html.haml        |  7 +---
 app/views/profiles/passwords/new.html.haml         |  7 +---
 app/views/profiles/show.html.haml                  |  7 +---
 app/views/projects/_errors.html.haml               |  5 +--
 app/views/projects/deploy_keys/_form.html.haml     |  6 +--
 app/views/projects/hooks/index.html.haml           |  6 +--
 app/views/projects/labels/_form.html.haml          |  8 +---
 .../projects/merge_requests/_new_compare.html.haml |  5 +--
 app/views/projects/milestones/_form.html.haml      |  7 +---
 .../projects/protected_branches/index.html.haml    |  6 +--
 app/views/projects/variables/show.html.haml        |  8 +---
 app/views/projects/wikis/_form.html.haml           |  6 +--
 app/views/shared/_service_settings.html.haml       |  7 +---
 app/views/shared/issuable/_form.html.haml          |  9 +----
 app/views/shared/snippets/_form.html.haml          |  6 +--
 spec/helpers/form_helper_spec.rb                   | 46 ++++++++++++++++++++++
 33 files changed, 105 insertions(+), 153 deletions(-)
 create mode 100644 app/helpers/form_helper.rb
 create mode 100644 spec/helpers/form_helper_spec.rb

diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb
new file mode 100644
index 00000000000..6a43be2cf3e
--- /dev/null
+++ b/app/helpers/form_helper.rb
@@ -0,0 +1,18 @@
+module FormHelper
+  def form_errors(model)
+    return unless model.errors.any?
+
+    pluralized = 'error'.pluralize(model.errors.count)
+    headline   = "The form contains the following #{pluralized}:"
+
+    content_tag(:div, class: 'alert alert-danger', id: 'error_explanation') do
+      content_tag(:h4, headline) <<
+      content_tag(:ul) do
+        model.errors.full_messages.
+          map { |msg| content_tag(:li, msg) }.
+          join.
+          html_safe
+      end
+    end
+  end
+end
diff --git a/app/views/abuse_reports/new.html.haml b/app/views/abuse_reports/new.html.haml
index 3bc1b24b5e2..06be1a53318 100644
--- a/app/views/abuse_reports/new.html.haml
+++ b/app/views/abuse_reports/new.html.haml
@@ -3,11 +3,9 @@
 %p Please use this form to report users who create spam issues, comments or behave inappropriately.
 %hr
 = form_for @abuse_report, html: { class: 'form-horizontal js-quick-submit js-requires-input'} do |f|
+  = form_errors(@abuse_report)
+
   = f.hidden_field :user_id
-  - if @abuse_report.errors.any?
-    .alert.alert-danger
-      - @abuse_report.errors.full_messages.each do |msg|
-        %p= msg
   .form-group
     = f.label :user_id, class: 'control-label'
     .col-sm-10
diff --git a/app/views/admin/appearances/_form.html.haml b/app/views/admin/appearances/_form.html.haml
index 6f325914d14..d88f3ad314d 100644
--- a/app/views/admin/appearances/_form.html.haml
+++ b/app/views/admin/appearances/_form.html.haml
@@ -1,8 +1,5 @@
 = form_for @appearance, url: admin_appearances_path, html: { class: 'form-horizontal'} do |f|
-  - if @appearance.errors.any?
-    .alert.alert-danger
-      - @appearance.errors.full_messages.each do |msg|
-        %p= msg
+  = form_errors(@appearance)
 
   %fieldset.sign-in
     %legend
diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml
index de86dacbb12..a8cca1a81cb 100644
--- a/app/views/admin/application_settings/_form.html.haml
+++ b/app/views/admin/application_settings/_form.html.haml
@@ -1,9 +1,5 @@
 = form_for @application_setting, url: admin_application_settings_path, html: { class: 'form-horizontal fieldset-form' } do |f|
-  - if @application_setting.errors.any?
-    #error_explanation
-      .alert.alert-danger
-        - @application_setting.errors.full_messages.each do |msg|
-          %p= msg
+  = form_errors(@application_setting)
 
   %fieldset
     %legend Visibility and Access Controls
diff --git a/app/views/admin/applications/_form.html.haml b/app/views/admin/applications/_form.html.haml
index e18f7b499dd..4aacbb8cd77 100644
--- a/app/views/admin/applications/_form.html.haml
+++ b/app/views/admin/applications/_form.html.haml
@@ -1,9 +1,6 @@
 = form_for [:admin, @application], url: @url, html: {class: 'form-horizontal', role: 'form'} do |f|
-  - if application.errors.any?
-    .alert.alert-danger
-      %button{ type: "button", class: "close", "data-dismiss" => "alert"} &times;
-      - application.errors.full_messages.each do |msg|
-        %p= msg
+  = form_errors(application)
+
   = content_tag :div, class: 'form-group' do
     = f.label :name, class: 'col-sm-2 control-label'
     .col-sm-10
diff --git a/app/views/admin/broadcast_messages/_form.html.haml b/app/views/admin/broadcast_messages/_form.html.haml
index b748460a9f7..6b157abf842 100644
--- a/app/views/admin/broadcast_messages/_form.html.haml
+++ b/app/views/admin/broadcast_messages/_form.html.haml
@@ -4,10 +4,8 @@
     = render_broadcast_message(@broadcast_message.message.presence || "Your message here")
 
 = form_for [:admin, @broadcast_message], html: { class: 'broadcast-message-form form-horizontal js-quick-submit js-requires-input'} do |f|
-  -if @broadcast_message.errors.any?
-    .alert.alert-danger
-      - @broadcast_message.errors.full_messages.each do |msg|
-        %p= msg
+  = form_errors(@broadcast_message)
+
   .form-group
     = f.label :message, class: 'control-label'
     .col-sm-10
diff --git a/app/views/admin/deploy_keys/new.html.haml b/app/views/admin/deploy_keys/new.html.haml
index 5b46b3222a9..15aa059c93d 100644
--- a/app/views/admin/deploy_keys/new.html.haml
+++ b/app/views/admin/deploy_keys/new.html.haml
@@ -4,11 +4,7 @@
 
 %div
   = form_for [:admin, @deploy_key], html: { class: 'deploy-key-form form-horizontal' } do |f|
-    -if @deploy_key.errors.any?
-      .alert.alert-danger
-        %ul
-          - @deploy_key.errors.full_messages.each do |msg|
-            %li= msg
+    = form_errors(@deploy_key)
 
     .form-group
       = f.label :title, class: "control-label"
diff --git a/app/views/admin/groups/_form.html.haml b/app/views/admin/groups/_form.html.haml
index 7f2b1cd235d..0cc405401cf 100644
--- a/app/views/admin/groups/_form.html.haml
+++ b/app/views/admin/groups/_form.html.haml
@@ -1,8 +1,5 @@
 = form_for [:admin, @group], html: { class: "form-horizontal" } do |f|
-  - if @group.errors.any?
-    .alert.alert-danger
-      %span= @group.errors.full_messages.first
-
+  = form_errors(@group)
   = render 'shared/group_form', f: f
 
   .form-group.group-description-holder
diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml
index 53b3cd04c68..ad952052f25 100644
--- a/app/views/admin/hooks/index.html.haml
+++ b/app/views/admin/hooks/index.html.haml
@@ -10,10 +10,8 @@
 
 
 = form_for @hook, as: :hook, url: admin_hooks_path, html: { class: 'form-horizontal' } do |f|
-  -if @hook.errors.any?
-    .alert.alert-danger
-      - @hook.errors.full_messages.each do |msg|
-        %p= msg
+  = form_errors(@hook)
+
   .form-group
     = f.label :url, "URL:", class: 'control-label'
     .col-sm-10
diff --git a/app/views/admin/identities/_form.html.haml b/app/views/admin/identities/_form.html.haml
index 3a788558226..112a201fafa 100644
--- a/app/views/admin/identities/_form.html.haml
+++ b/app/views/admin/identities/_form.html.haml
@@ -1,9 +1,5 @@
 = form_for [:admin, @user, @identity], html: { class: 'form-horizontal fieldset-form' } do |f|
-  - if @identity.errors.any?
-    #error_explanation
-      .alert.alert-danger
-        - @identity.errors.full_messages.each do |msg|
-          %p= msg
+  = form_errors(@identity)
 
   .form-group
     = f.label :provider, class: 'control-label'
diff --git a/app/views/admin/labels/_form.html.haml b/app/views/admin/labels/_form.html.haml
index 8c6b389bf15..448aa953548 100644
--- a/app/views/admin/labels/_form.html.haml
+++ b/app/views/admin/labels/_form.html.haml
@@ -1,11 +1,5 @@
 = form_for [:admin, @label], html: { class: 'form-horizontal label-form js-requires-input' } do |f|
-  -if @label.errors.any?
-    .row
-      .col-sm-offset-2.col-sm-10
-        .alert.alert-danger
-          - @label.errors.full_messages.each do |msg|
-            %span= msg
-            %br
+  = form_errors(@label)
 
   .form-group
     = f.label :title, class: 'control-label'
diff --git a/app/views/admin/users/_form.html.haml b/app/views/admin/users/_form.html.haml
index d2527ede995..b05fdbd5552 100644
--- a/app/views/admin/users/_form.html.haml
+++ b/app/views/admin/users/_form.html.haml
@@ -1,10 +1,6 @@
 .user_new
   = form_for [:admin, @user], html: { class: 'form-horizontal fieldset-form' } do |f|
-    -if @user.errors.any?
-      #error_explanation
-        .alert.alert-danger
-          - @user.errors.full_messages.each do |msg|
-            %p= msg
+    = form_errors(@user)
 
     %fieldset
       %legend Account
diff --git a/app/views/doorkeeper/applications/_form.html.haml b/app/views/doorkeeper/applications/_form.html.haml
index 906b0676150..5c98265727a 100644
--- a/app/views/doorkeeper/applications/_form.html.haml
+++ b/app/views/doorkeeper/applications/_form.html.haml
@@ -1,9 +1,5 @@
 = form_for application, url: doorkeeper_submit_path(application), html: {role: 'form'} do |f|
-  - if application.errors.any?
-    .alert.alert-danger
-      %ul
-        - application.errors.full_messages.each do |msg|
-          %li= msg
+  = form_errors(application)
 
   .form-group
     = f.label :name, class: 'label-light'
diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml
index ea5a0358392..a698cbbe9db 100644
--- a/app/views/groups/edit.html.haml
+++ b/app/views/groups/edit.html.haml
@@ -5,9 +5,7 @@
     Group settings
   .panel-body
     = form_for @group, html: { multipart: true, class: "form-horizontal" }, authenticity_token: true do |f|
-      - if @group.errors.any?
-        .alert.alert-danger
-          %span= @group.errors.full_messages.first
+      = form_errors(@group)
       = render 'shared/group_form', f: f
 
       .form-group
diff --git a/app/views/groups/new.html.haml b/app/views/groups/new.html.haml
index 30ab8aeba13..2b8bc269e64 100644
--- a/app/views/groups/new.html.haml
+++ b/app/views/groups/new.html.haml
@@ -6,10 +6,7 @@
 %hr
 
 = form_for @group, html: { class: 'group-form form-horizontal' } do |f|
-  - if @group.errors.any?
-    .alert.alert-danger
-      %span= @group.errors.full_messages.first
-
+  = form_errors(@group)
   = render 'shared/group_form', f: f, autofocus: true
 
   .form-group.group-description-holder
diff --git a/app/views/profiles/keys/_form.html.haml b/app/views/profiles/keys/_form.html.haml
index 4d78215ed3c..b3ed59a1a4a 100644
--- a/app/views/profiles/keys/_form.html.haml
+++ b/app/views/profiles/keys/_form.html.haml
@@ -1,10 +1,6 @@
 %div
   = form_for [:profile, @key], html: { class: 'js-requires-input' } do |f|
-    - if @key.errors.any?
-      .alert.alert-danger
-        %ul
-          - @key.errors.full_messages.each do |msg|
-            %li= msg
+    = form_errors(@key)
 
     .form-group
       = f.label :key, class: 'label-light'
diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml
index 3d15c0d932b..6609295a2a5 100644
--- a/app/views/profiles/notifications/show.html.haml
+++ b/app/views/profiles/notifications/show.html.haml
@@ -2,11 +2,7 @@
 - header_title page_title, profile_notifications_path
 
 = form_for @user, url: profile_notifications_path, method: :put, html: { class: 'update-notifications prepend-top-default' } do |f|
-  -if @user.errors.any?
-    %div.alert.alert-danger
-      %ul
-        - @user.errors.full_messages.each do |msg|
-          %li= msg
+  = form_errors(@user)
 
   = hidden_field_tag :notification_type, 'global'
   .row
diff --git a/app/views/profiles/passwords/edit.html.haml b/app/views/profiles/passwords/edit.html.haml
index 44d758dceb3..5ac8a8b9d09 100644
--- a/app/views/profiles/passwords/edit.html.haml
+++ b/app/views/profiles/passwords/edit.html.haml
@@ -13,11 +13,8 @@
       - unless @user.password_automatically_set?
         or recover your current one
     = form_for @user, url: profile_password_path, method: :put, html: {class: "update-password"} do |f|
-      -if @user.errors.any?
-        .alert.alert-danger
-          %ul
-            - @user.errors.full_messages.each do |msg|
-              %li= msg
+      = form_errors(@user)
+
       - unless @user.password_automatically_set?
         .form-group
           = f.label :current_password, class: 'label-light'
diff --git a/app/views/profiles/passwords/new.html.haml b/app/views/profiles/passwords/new.html.haml
index d165f758c81..2eb9fac57c3 100644
--- a/app/views/profiles/passwords/new.html.haml
+++ b/app/views/profiles/passwords/new.html.haml
@@ -7,11 +7,8 @@
     Please set a new password before proceeding.
     %br
     After a successful password update you will be redirected to login screen.
-  -if @user.errors.any?
-    .alert.alert-danger
-      %ul
-        - @user.errors.full_messages.each do |msg|
-          %li= msg
+
+  = form_errors(@user)
 
   - unless @user.password_automatically_set?
     .form-group
diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml
index dcb3be9585d..f59d27f7ed0 100644
--- a/app/views/profiles/show.html.haml
+++ b/app/views/profiles/show.html.haml
@@ -1,9 +1,6 @@
 = form_for @user, url: profile_path, method: :put, html: { multipart: true, class: "edit-user prepend-top-default" }, authenticity_token: true do |f|
-  -if @user.errors.any?
-    %div.alert.alert-danger
-      %ul
-        - @user.errors.full_messages.each do |msg|
-          %li= msg
+  = form_errors(@user)
+
   .row
     .col-lg-3.profile-settings-sidebar
       %h4.prepend-top-0
diff --git a/app/views/projects/_errors.html.haml b/app/views/projects/_errors.html.haml
index 7c8bb33ed7e..2dba22d3be6 100644
--- a/app/views/projects/_errors.html.haml
+++ b/app/views/projects/_errors.html.haml
@@ -1,4 +1 @@
-- if @project.errors.any?
-  .alert.alert-danger
-    %button{ type: "button", class: "close", "data-dismiss" => "alert"} &times;
-    = @project.errors.full_messages.first
+= form_errors(@project)
diff --git a/app/views/projects/deploy_keys/_form.html.haml b/app/views/projects/deploy_keys/_form.html.haml
index 5e182af2669..f6565f85836 100644
--- a/app/views/projects/deploy_keys/_form.html.haml
+++ b/app/views/projects/deploy_keys/_form.html.haml
@@ -1,10 +1,6 @@
 %div
   = form_for [@project.namespace.becomes(Namespace), @project, @key], url: namespace_project_deploy_keys_path, html: { class: 'deploy-key-form form-horizontal js-requires-input' } do |f|
-    -if @key.errors.any?
-      .alert.alert-danger
-        %ul
-          - @key.errors.full_messages.each do |msg|
-            %li= msg
+    = form_errors(@key)
 
     .form-group
       = f.label :title, class: "control-label"
diff --git a/app/views/projects/hooks/index.html.haml b/app/views/projects/hooks/index.html.haml
index 67d016bd871..e39224d86c6 100644
--- a/app/views/projects/hooks/index.html.haml
+++ b/app/views/projects/hooks/index.html.haml
@@ -9,10 +9,8 @@
 %hr.clearfix
 
 = form_for [@project.namespace.becomes(Namespace), @project, @hook], as: :hook, url: namespace_project_hooks_path(@project.namespace, @project), html: { class: 'form-horizontal' } do |f|
-  -if @hook.errors.any?
-    .alert.alert-danger
-      - @hook.errors.full_messages.each do |msg|
-        %p= msg
+  = form_errors(@hook)
+
   .form-group
     = f.label :url, "URL", class: 'control-label'
     .col-sm-10
diff --git a/app/views/projects/labels/_form.html.haml b/app/views/projects/labels/_form.html.haml
index be7a0bb5628..aa143e54ffe 100644
--- a/app/views/projects/labels/_form.html.haml
+++ b/app/views/projects/labels/_form.html.haml
@@ -1,11 +1,5 @@
 = form_for [@project.namespace.becomes(Namespace), @project, @label], html: { class: 'form-horizontal label-form js-quick-submit js-requires-input' } do |f|
-  -if @label.errors.any?
-    .row
-      .col-sm-offset-2.col-sm-10
-        .alert.alert-danger
-          - @label.errors.full_messages.each do |msg|
-            %span= msg
-            %br
+  = form_errors(@label)
 
   .form-group
     = f.label :title, class: 'control-label'
diff --git a/app/views/projects/merge_requests/_new_compare.html.haml b/app/views/projects/merge_requests/_new_compare.html.haml
index 01dc7519bee..0931f743a35 100644
--- a/app/views/projects/merge_requests/_new_compare.html.haml
+++ b/app/views/projects/merge_requests/_new_compare.html.haml
@@ -28,10 +28,7 @@
           .mr_target_commit
 
   - if @merge_request.errors.any?
-    .alert.alert-danger
-      - @merge_request.errors.full_messages.each do |msg|
-        %div= msg
-
+    = form_errors(@merge_request)
   - elsif @merge_request.source_branch.present? && @merge_request.target_branch.present?
     .light-well.append-bottom-default
       .center
diff --git a/app/views/projects/milestones/_form.html.haml b/app/views/projects/milestones/_form.html.haml
index 23f2bca7baf..b2dae1c70ee 100644
--- a/app/views/projects/milestones/_form.html.haml
+++ b/app/views/projects/milestones/_form.html.haml
@@ -1,9 +1,6 @@
 = form_for [@project.namespace.becomes(Namespace), @project, @milestone], html: {class: 'form-horizontal milestone-form gfm-form js-quick-submit js-requires-input'}  do |f|
-  -if @milestone.errors.any?
-    .alert.alert-danger
-      %ul
-        - @milestone.errors.full_messages.each do |msg|
-          %li= msg
+  = form_errors(@milestone)
+
   .row
     .col-md-6
       .form-group
diff --git a/app/views/projects/protected_branches/index.html.haml b/app/views/projects/protected_branches/index.html.haml
index cfd7e1534ca..653b02da4db 100644
--- a/app/views/projects/protected_branches/index.html.haml
+++ b/app/views/projects/protected_branches/index.html.haml
@@ -13,11 +13,7 @@
 
 - if can? current_user, :admin_project, @project
   = form_for [@project.namespace.becomes(Namespace), @project, @protected_branch], html: { class: 'form-horizontal' } do |f|
-    -if @protected_branch.errors.any?
-      .alert.alert-danger
-        %ul
-          - @protected_branch.errors.full_messages.each do |msg|
-            %li= msg
+    = form_errors(@protected_branch)
 
     .form-group
       = f.label :name, "Branch", class: 'control-label'
diff --git a/app/views/projects/variables/show.html.haml b/app/views/projects/variables/show.html.haml
index efe1e6f24c2..ca284b84d39 100644
--- a/app/views/projects/variables/show.html.haml
+++ b/app/views/projects/variables/show.html.haml
@@ -13,13 +13,7 @@
 
 
 = nested_form_for @project, url: url_for(controller: 'projects/variables', action: 'update'), html: { class: 'form-horizontal' }  do |f|
-  - if @project.errors.any?
-    #error_explanation
-      %p.lead= "#{pluralize(@project.errors.count, "error")} prohibited this project from being saved:"
-      .alert.alert-error
-        %ul
-          - @project.errors.full_messages.each do |msg|
-            %li= msg
+  = form_errors(@project)
 
   = f.fields_for :variables do |variable_form|
     .form-group
diff --git a/app/views/projects/wikis/_form.html.haml b/app/views/projects/wikis/_form.html.haml
index f0d1932e23c..812876e2835 100644
--- a/app/views/projects/wikis/_form.html.haml
+++ b/app/views/projects/wikis/_form.html.haml
@@ -1,9 +1,5 @@
 = form_for [@project.namespace.becomes(Namespace), @project, @page], method: @page.persisted? ? :put : :post, html: { class: 'form-horizontal wiki-form gfm-form prepend-top-default js-quick-submit' } do |f|
-  -if @page.errors.any?
-    #error_explanation
-      .alert.alert-danger
-        - @page.errors.full_messages.each do |msg|
-          %p= msg
+  = form_errors(@page)
 
   = f.hidden_field :title, value: @page.title
   .form-group
diff --git a/app/views/shared/_service_settings.html.haml b/app/views/shared/_service_settings.html.haml
index 5a60ff5a5da..fc935166bf6 100644
--- a/app/views/shared/_service_settings.html.haml
+++ b/app/views/shared/_service_settings.html.haml
@@ -1,9 +1,4 @@
-- if @service.errors.any?
-  #error_explanation
-    .alert.alert-danger
-      %ul
-        - @service.errors.full_messages.each do |msg|
-          %li= msg
+= form_errors(@service)
 
 - if @service.help.present?
   .well
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index e2a9e5bfb92..0cda785f91e 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -1,10 +1,5 @@
-- if issuable.errors.any?
-  .row
-    .col-sm-offset-2.col-sm-10
-      .alert.alert-danger
-        - issuable.errors.full_messages.each do |msg|
-          %span= msg
-          %br
+= form_errors(issuable)
+
 .form-group
   = f.label :title, class: 'control-label'
   .col-sm-10
diff --git a/app/views/shared/snippets/_form.html.haml b/app/views/shared/snippets/_form.html.haml
index 1041eccd1df..47ec09f62c6 100644
--- a/app/views/shared/snippets/_form.html.haml
+++ b/app/views/shared/snippets/_form.html.haml
@@ -1,10 +1,6 @@
 .snippet-form-holder
   = form_for @snippet, url: url, html: { class: "form-horizontal snippet-form js-requires-input" } do |f|
-    - if @snippet.errors.any?
-      .alert.alert-danger
-        %ul
-          - @snippet.errors.full_messages.each do |msg|
-            %li= msg
+    = form_errors(@snippet)
 
     .form-group
       = f.label :title, class: 'control-label'
diff --git a/spec/helpers/form_helper_spec.rb b/spec/helpers/form_helper_spec.rb
new file mode 100644
index 00000000000..b20373a96fb
--- /dev/null
+++ b/spec/helpers/form_helper_spec.rb
@@ -0,0 +1,46 @@
+require 'rails_helper'
+
+describe FormHelper do
+  describe 'form_errors' do
+    it 'returns nil when model has no errors' do
+      model = double(errors: [])
+
+      expect(helper.form_errors(model)).to be_nil
+    end
+
+    it 'renders an alert div' do
+      model = double(errors: errors_stub('Error 1'))
+
+      expect(helper.form_errors(model)).
+        to include('<div class="alert alert-danger" id="error_explanation">')
+    end
+
+    it 'contains a summary message' do
+      single_error = double(errors: errors_stub('A'))
+      multi_errors = double(errors: errors_stub('A', 'B', 'C'))
+
+      expect(helper.form_errors(single_error)).
+        to include('<h4>The form contains the following error:')
+      expect(helper.form_errors(multi_errors)).
+        to include('<h4>The form contains the following errors:')
+    end
+
+    it 'renders each message' do
+      model = double(errors: errors_stub('Error 1', 'Error 2', 'Error 3'))
+
+      errors = helper.form_errors(model)
+
+      aggregate_failures do
+        expect(errors).to include('<li>Error 1</li>')
+        expect(errors).to include('<li>Error 2</li>')
+        expect(errors).to include('<li>Error 3</li>')
+      end
+    end
+
+    def errors_stub(*messages)
+      ActiveModel::Errors.new(double).tap do |errors|
+        messages.each { |msg| errors.add(:base, msg) }
+      end
+    end
+  end
+end
-- 
cgit v1.2.1


From 831d1807df6d1d9f8213898614a24bea78208114 Mon Sep 17 00:00:00 2001
From: "P.S.V.R" <pmq2001@gmail.com>
Date: Tue, 5 Apr 2016 16:20:08 +0800
Subject: Eliminate ugly scroll bars shown in modal boxes

---
 app/assets/stylesheets/pages/help.scss | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/app/assets/stylesheets/pages/help.scss b/app/assets/stylesheets/pages/help.scss
index bd224705f04..604f1700cf8 100644
--- a/app/assets/stylesheets/pages/help.scss
+++ b/app/assets/stylesheets/pages/help.scss
@@ -59,6 +59,9 @@
   position: relative;
   overflow-y: auto;
   padding: 15px;
+  .form-actions {
+    margin: -$gl-padding+1;
+  }
 }
 
 body.modal-open {
-- 
cgit v1.2.1


From c5c05f6a04ab3c791bc7c34dc74925731cf2ff94 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Tue, 15 Mar 2016 14:43:40 +0000
Subject: Updated UI for new merge request

Closes #2540
---
 app/assets/javascripts/compare.js.coffee           | 61 +++++++++++++++++++
 app/assets/stylesheets/pages/merge_requests.scss   | 51 ++++++++++++++--
 .../projects/merge_requests_controller.rb          |  2 +
 app/helpers/commits_helper.rb                      |  4 +-
 app/views/projects/commits/_commit.html.haml       |  2 +-
 .../projects/merge_requests/_new_compare.html.haml | 71 +++++++---------------
 .../projects/merge_requests/branch_from.html.haml  |  1 +
 .../projects/merge_requests/branch_from.js.haml    |  3 -
 .../projects/merge_requests/branch_to.html.haml    |  1 +
 .../projects/merge_requests/branch_to.js.haml      |  3 -
 10 files changed, 138 insertions(+), 61 deletions(-)
 create mode 100644 app/assets/javascripts/compare.js.coffee
 create mode 100644 app/views/projects/merge_requests/branch_from.html.haml
 delete mode 100644 app/views/projects/merge_requests/branch_from.js.haml
 create mode 100644 app/views/projects/merge_requests/branch_to.html.haml
 delete mode 100644 app/views/projects/merge_requests/branch_to.js.haml

diff --git a/app/assets/javascripts/compare.js.coffee b/app/assets/javascripts/compare.js.coffee
new file mode 100644
index 00000000000..c13744ebc62
--- /dev/null
+++ b/app/assets/javascripts/compare.js.coffee
@@ -0,0 +1,61 @@
+class @Compare
+  constructor: (@opts) ->
+    @source_loading = $ ".js-source-loading"
+    @target_loading = $ ".js-target-loading"
+    @source_branch = $ "#merge_request_source_branch"
+    @target_branch = $ "#merge_request_target_branch"
+    @target_project = $ "#merge_request_target_project_id"
+
+    @initialState()
+    @cleanBinding()
+    @addBinding()
+
+  cleanBinding: ->
+    @source_branch.off "change"
+    @target_branch.off "change"
+    @target_project.off "change"
+
+  addBinding: ->
+    @source_branch.on "change", =>
+      @getSourceHtml()
+    @target_branch.on "change", =>
+      @getTargetHtml()
+    @target_project.on "change", =>
+      @getTargetProject()
+
+  initialState: ->
+    @getSourceHtml()
+    @getTargetHtml()
+
+  getTargetProject: ->
+    $.get @opts.targetProjectUrl,
+      target_project_id:  @target_project.val()
+
+  getSourceHtml: ->
+    $.ajax(
+      url: @opts.sourceBranchUrl
+      data:
+        ref: @source_branch.val()
+      beforeSend: =>
+        @source_loading.show()
+        $(".mr_source_commit").html ""
+      success: (html) =>
+        @source_loading.hide()
+        $(".mr_source_commit").html html
+        $(".mr_source_commit .js-timeago").timeago()
+    )
+
+  getTargetHtml: ->
+    $.ajax(
+      url: @opts.targetBranchUrl
+      data:
+        target_project_id: @target_project.val()
+        ref: @target_branch.val()
+      beforeSend: =>
+        @target_loading.show()
+        $(".mr_target_commit").html ""
+      success: (html) =>
+        @target_loading.hide()
+        $(".mr_target_commit").html html
+        $(".mr_target_commit .js-timeago").timeago()
+    )
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 1c6a4208974..7bc1f58471f 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -123,6 +123,8 @@
 
 .mr_source_commit,
 .mr_target_commit {
+  margin-bottom: 0;
+
   .commit {
     margin: 0;
     padding: 2px 0;
@@ -174,10 +176,6 @@
   display: none;
 }
 
-.merge-request-form .select2-container {
-  width: 250px !important;
-}
-
 #modal_merge_info .modal-dialog {
   width: 600px;
 
@@ -200,3 +198,48 @@
     overflow-x: scroll;
   }
 }
+
+.panel-new-merge-request {
+  .panel-heading {
+    padding: 5px 10px;
+    font-weight: 600;
+    line-height: 25px;
+  }
+
+  .panel-body {
+    padding: 10px 5px;
+  }
+
+  .panel-footer {
+    padding: 10px 10px;
+  }
+
+  .commit {
+    .commit-row-title {
+      margin-bottom: 4px;
+    }
+
+    .avatar {
+      width: 20px;
+      height: 20px;
+      margin-right: 5px;
+    }
+
+    .commit-row-info {
+      line-height: 20px;
+    }
+  }
+
+  .btn-clipboard {
+    margin-right: 5px;
+    padding: 0;
+    background: transparent;
+  }
+}
+
+.merge-request-select {
+  float: left;
+  width: 50%;
+  padding-left: 5px;
+  padding-right: 5px;
+}
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 49064f5d505..ca6cfc99143 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -207,11 +207,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     #This is always source
     @source_project = @merge_request.nil? ? @project : @merge_request.source_project
     @commit = @repository.commit(params[:ref]) if params[:ref].present?
+    render layout: false
   end
 
   def branch_to
     @target_project = selected_target_project
     @commit = @target_project.commit(params[:ref]) if params[:ref].present?
+    render layout: false
   end
 
   def update_branches
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index bde0799f3de..a65e2e5cb8f 100644
--- a/app/helpers/commits_helper.rb
+++ b/app/helpers/commits_helper.rb
@@ -28,7 +28,7 @@ module CommitsHelper
 
   def commit_to_html(commit, project, inline = true)
     template = inline ? "inline_commit" : "commit"
-    escape_javascript(render "projects/commits/#{template}", commit: commit, project: project) unless commit.nil?
+    render "projects/commits/#{template}", commit: commit, project: project unless commit.nil?
   end
 
   # Breadcrumb links for a Project and, if applicable, a tree path
@@ -117,7 +117,7 @@ module CommitsHelper
       end
     end
     link_to(
-      "Browse Files »",
+      "Browse Files",
       namespace_project_tree_path(project.namespace, project, commit),
       class: "pull-right"
     )
diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml
index 7f2903589a9..b55fe510f70 100644
--- a/app/views/projects/commits/_commit.html.haml
+++ b/app/views/projects/commits/_commit.html.haml
@@ -35,8 +35,8 @@
           = preserve(markdown(escape_once(commit.description), pipeline: :single_line))
 
     .commit-row-info
+      by 
       = commit_author_link(commit, avatar: true, size: 24)
-      authored
       .committed_ago
         #{time_ago_with_tooltip(commit.committed_date, skip_js: true)} &nbsp;
       = link_to_browse_code(project, commit)
diff --git a/app/views/projects/merge_requests/_new_compare.html.haml b/app/views/projects/merge_requests/_new_compare.html.haml
index 01dc7519bee..4fc74dfcf45 100644
--- a/app/views/projects/merge_requests/_new_compare.html.haml
+++ b/app/views/projects/merge_requests/_new_compare.html.haml
@@ -5,27 +5,31 @@
   .hide.alert.alert-danger.mr-compare-errors
   .merge-request-branches.row
     .col-md-6
-      .panel.panel-default
+      .panel.panel-default.panel-new-merge-request
         .panel-heading
-          %strong Source branch
-        .panel-body
-          = f.select(:source_project_id, [[@merge_request.source_project_path,@merge_request.source_project.id]] , {}, { class: 'source_project select2 span3', disabled: @merge_request.persisted?, required: true })
-          &nbsp;
-          = f.select(:source_branch, @merge_request.source_branches, { include_blank: true }, { class: 'source_branch select2 span2', required: true, data: { placeholder: "Select source branch" } })
+          Source branch
+        .panel-body.clearfix
+          .merge-request-select
+            = f.select(:source_project_id, [[@merge_request.source_project_path,@merge_request.source_project.id]] , {}, { class: 'source_project select2', disabled: @merge_request.persisted?, required: true })
+          .merge-request-select
+            = f.select(:source_branch, @merge_request.source_branches, { include_blank: true }, { class: 'source_branch select2', required: true, data: { placeholder: "Select source branch" } })
         .panel-footer
-          .mr_source_commit
+          = icon('spinner spin', class: "js-source-loading")
+          %ul.list-unstyled.mr_source_commit
 
     .col-md-6
-      .panel.panel-default
+      .panel.panel-default.panel-new-merge-request
         .panel-heading
-          %strong Target branch
-        .panel-body
+          Target branch
+        .panel-body.clearfix
           - projects =  @project.forked_from_project.nil? ? [@project] : [@project, @project.forked_from_project]
-          = f.select(:target_project_id, options_from_collection_for_select(projects, 'id', 'path_with_namespace', f.object.target_project_id), {}, { class: 'target_project select2 span3', disabled: @merge_request.persisted?, required: true })
-          &nbsp;
-          = f.select(:target_branch, @merge_request.target_branches, { include_blank: true }, { class: 'target_branch select2 span2', required: true, data: { placeholder: "Select target branch" } })
+          .merge-request-select
+            = f.select(:target_project_id, options_from_collection_for_select(projects, 'id', 'path_with_namespace', f.object.target_project_id), {}, { class: 'target_project select2', disabled: @merge_request.persisted?, required: true })
+          .merge-request-select
+            = f.select(:target_branch, @merge_request.target_branches, { include_blank: true }, { class: 'target_branch select2', required: true, data: { placeholder: "Select target branch" } })
         .panel-footer
-          .mr_target_commit
+          = icon('spinner spin', class: "js-target-loading")
+          %ul.list-unstyled.mr_target_commit
 
   - if @merge_request.errors.any?
     .alert.alert-danger
@@ -45,40 +49,11 @@
             and
             %span.label-branch #{@merge_request.target_branch}
             are the same.
-
-
-  .form-actions
-    = f.submit 'Compare branches and continue', class: "btn btn-new mr-compare-btn"
-
-:javascript
-  var source_branch = $("#merge_request_source_branch")
-    , target_branch = $("#merge_request_target_branch")
-    , target_project = $("#merge_request_target_project_id");
-
-  $.get("#{branch_from_namespace_project_merge_requests_path(@source_project.namespace, @source_project)}", {ref: source_branch.val() });
-  $.get("#{branch_to_namespace_project_merge_requests_path(@source_project.namespace, @source_project)}", {target_project_id: target_project.val(),ref: target_branch.val() });
-
-  target_project.on("change", function() {
-    $.get("#{update_branches_namespace_project_merge_requests_path(@source_project.namespace, @source_project)}", {target_project_id:  $(this).val() });
-  });
-  source_branch.on("change", function() {
-    $.get("#{branch_from_namespace_project_merge_requests_path(@source_project.namespace, @source_project)}", {ref: $(this).val() });
-    $(".mr-compare-errors").fadeOut();
-    $(".mr-compare-btn").enable();
-  });
-  target_branch.on("change", function() {
-    $.get("#{branch_to_namespace_project_merge_requests_path(@source_project.namespace, @source_project)}", {target_project_id: target_project.val(),ref: $(this).val() });
-    $(".mr-compare-errors").fadeOut();
-    $(".mr-compare-btn").enable();
-  });
-
+  = f.submit 'Compare branches and continue', class: "btn btn-new mr-compare-btn"
 
 :javascript
-  $(".merge-request-form").on('submit', function () {
-    if ($("#merge_request_source_branch").val() === "" || $('#merge_request_target_branch').val() === "") {
-      $(".mr-compare-errors").html("You must select source and target branch to proceed");
-      $(".mr-compare-errors").fadeIn();
-      event.preventDefault();
-      return;
-    }
+  new Compare({
+    targetProjectUrl: "#{update_branches_namespace_project_merge_requests_path(@source_project.namespace, @source_project)}",
+    sourceBranchUrl: "#{branch_from_namespace_project_merge_requests_path(@source_project.namespace, @source_project)}",
+    targetBranchUrl: "#{branch_to_namespace_project_merge_requests_path(@source_project.namespace, @source_project)}"
   });
diff --git a/app/views/projects/merge_requests/branch_from.html.haml b/app/views/projects/merge_requests/branch_from.html.haml
new file mode 100644
index 00000000000..4f90dde6fa8
--- /dev/null
+++ b/app/views/projects/merge_requests/branch_from.html.haml
@@ -0,0 +1 @@
+= commit_to_html(@commit, @source_project, false)
diff --git a/app/views/projects/merge_requests/branch_from.js.haml b/app/views/projects/merge_requests/branch_from.js.haml
deleted file mode 100644
index 9210798f39c..00000000000
--- a/app/views/projects/merge_requests/branch_from.js.haml
+++ /dev/null
@@ -1,3 +0,0 @@
-:plain
-  $(".mr_source_commit").html("#{commit_to_html(@commit, @source_project, false)}");
-  $('.js-timeago').timeago()
diff --git a/app/views/projects/merge_requests/branch_to.html.haml b/app/views/projects/merge_requests/branch_to.html.haml
new file mode 100644
index 00000000000..67a7a6bcec9
--- /dev/null
+++ b/app/views/projects/merge_requests/branch_to.html.haml
@@ -0,0 +1 @@
+= commit_to_html(@commit, @target_project, false)
diff --git a/app/views/projects/merge_requests/branch_to.js.haml b/app/views/projects/merge_requests/branch_to.js.haml
deleted file mode 100644
index 32fe2d535f3..00000000000
--- a/app/views/projects/merge_requests/branch_to.js.haml
+++ /dev/null
@@ -1,3 +0,0 @@
-:plain
-  $(".mr_target_commit").html("#{commit_to_html(@commit, @target_project, false)}");
-  $('.js-timeago').timeago()
-- 
cgit v1.2.1


From 7fe19046d6633d01f1a326a5cc0d80a24879f04d Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Tue, 29 Mar 2016 09:33:23 +0100
Subject: Fixed failing tests

---
 features/steps/project/source/browse_files.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index 243469b8e7d..5beb2cc6d76 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -213,8 +213,8 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
   end
 
   step 'I see Browse file link' do
-    expect(page).to have_link 'Browse File »'
-    expect(page).not_to have_link 'Browse Files »'
+    expect(page).to have_link 'Browse File'
+    expect(page).not_to have_link 'Browse Files'
   end
 
   step 'I see Browse code link' do
-- 
cgit v1.2.1


From b31005899858bb19f5de06b49621fa1d73dbe037 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Tue, 29 Mar 2016 11:11:16 +0100
Subject: Use new dropdowns for MR compare

---
 app/assets/javascripts/compare.js.coffee           | 43 ++++++++---------
 app/assets/javascripts/gl_dropdown.js.coffee       |  2 +-
 app/assets/stylesheets/pages/merge_requests.scss   | 10 ++++
 .../projects/merge_requests/_new_compare.html.haml | 55 ++++++++++++++++++----
 features/steps/project/source/browse_files.rb      |  4 +-
 5 files changed, 82 insertions(+), 32 deletions(-)

diff --git a/app/assets/javascripts/compare.js.coffee b/app/assets/javascripts/compare.js.coffee
index c13744ebc62..e5819fa91c3 100644
--- a/app/assets/javascripts/compare.js.coffee
+++ b/app/assets/javascripts/compare.js.coffee
@@ -2,26 +2,27 @@ class @Compare
   constructor: (@opts) ->
     @source_loading = $ ".js-source-loading"
     @target_loading = $ ".js-target-loading"
-    @source_branch = $ "#merge_request_source_branch"
-    @target_branch = $ "#merge_request_target_branch"
-    @target_project = $ "#merge_request_target_project_id"
 
-    @initialState()
-    @cleanBinding()
-    @addBinding()
+    $('.js-compare-dropdown').each (i, dropdown) =>
+      $dropdown = $(dropdown)
 
-  cleanBinding: ->
-    @source_branch.off "change"
-    @target_branch.off "change"
-    @target_project.off "change"
+      $dropdown.glDropdown(
+        selectable: true
+        fieldName: $dropdown.data 'field-name'
+        id: (obj, $el) ->
+          $el.data 'id'
+        toggleLabel: (obj, $el) ->
+          $el.text().trim()
+        clicked: (e, el) =>
+          if $dropdown.is '.js-target-branch'
+            @getTargetHtml()
+          else if $dropdown.is '.js-source-branch'
+            @getSourceHtml()
+          else if $dropdown.is '.js-target-project'
+            @getTargetProject()
+      )
 
-  addBinding: ->
-    @source_branch.on "change", =>
-      @getSourceHtml()
-    @target_branch.on "change", =>
-      @getTargetHtml()
-    @target_project.on "change", =>
-      @getTargetProject()
+    @initialState()
 
   initialState: ->
     @getSourceHtml()
@@ -29,13 +30,13 @@ class @Compare
 
   getTargetProject: ->
     $.get @opts.targetProjectUrl,
-      target_project_id:  @target_project.val()
+      target_project_id:  $("input[name='merge_request[source_project]']").val()
 
   getSourceHtml: ->
     $.ajax(
       url: @opts.sourceBranchUrl
       data:
-        ref: @source_branch.val()
+        ref: $("input[name='merge_request[source_branch]']").val()
       beforeSend: =>
         @source_loading.show()
         $(".mr_source_commit").html ""
@@ -49,8 +50,8 @@ class @Compare
     $.ajax(
       url: @opts.targetBranchUrl
       data:
-        target_project_id: @target_project.val()
-        ref: @target_branch.val()
+        target_project_id: $("input[name='merge_request[target_project_id]']").val()
+        ref: $("input[name='merge_request[target_branch]']").val()
       beforeSend: =>
         @target_loading.show()
         $(".mr_target_commit").html ""
diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee
index 2a4811b8763..38b38fc8426 100644
--- a/app/assets/javascripts/gl_dropdown.js.coffee
+++ b/app/assets/javascripts/gl_dropdown.js.coffee
@@ -373,7 +373,7 @@ class GitLabDropdown
 
       # Toggle the dropdown label
       if @options.toggleLabel
-        $(@el).find(".dropdown-toggle-text").text @options.toggleLabel(selectedObject)
+        $(@el).find(".dropdown-toggle-text").text @options.toggleLabel(selectedObject, el)
       if value?
         if !field.length
           # Create hidden input for form
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 7bc1f58471f..5929f607fc7 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -242,4 +242,14 @@
   width: 50%;
   padding-left: 5px;
   padding-right: 5px;
+
+  .dropdown-menu-toggle {
+    width: 100%;
+  }
+
+  .dropdown-menu {
+    left: 5px;
+    right: 5px;
+    width: auto;
+  }
 }
diff --git a/app/views/projects/merge_requests/_new_compare.html.haml b/app/views/projects/merge_requests/_new_compare.html.haml
index 4fc74dfcf45..f110c5f42c8 100644
--- a/app/views/projects/merge_requests/_new_compare.html.haml
+++ b/app/views/projects/merge_requests/_new_compare.html.haml
@@ -9,10 +9,29 @@
         .panel-heading
           Source branch
         .panel-body.clearfix
-          .merge-request-select
-            = f.select(:source_project_id, [[@merge_request.source_project_path,@merge_request.source_project.id]] , {}, { class: 'source_project select2', disabled: @merge_request.persisted?, required: true })
-          .merge-request-select
-            = f.select(:source_branch, @merge_request.source_branches, { include_blank: true }, { class: 'source_branch select2', required: true, data: { placeholder: "Select source branch" } })
+          .merge-request-select.dropdown
+            = f.hidden_field :source_project_id
+            = dropdown_toggle @merge_request.source_project_path, { toggle: "dropdown", field_name: "#{f.object_name}[source_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-source-project" }
+            .dropdown-menu
+              = dropdown_title("Select source project")
+              = dropdown_filter("Search projects")
+              = dropdown_content do
+                %ul
+                  %li
+                    %a{ href: "#", data: { id: @merge_request.source_project.id } }
+                      = @merge_request.source_project_path
+          .merge-request-select.dropdown
+            = f.hidden_field :source_branch
+            = dropdown_toggle "Select source branch", { toggle: "dropdown", field_name: "#{f.object_name}[source_branch]" }, { toggle_class: "js-compare-dropdown js-source-branch" }
+            .dropdown-menu
+              = dropdown_title("Select source branch")
+              = dropdown_filter("Search branches")
+              = dropdown_content do
+                %ul
+                  - @merge_request.source_branches.each do |branch|
+                    %li
+                      %a{ href: "#", data: { id: branch } }
+                        = branch
         .panel-footer
           = icon('spinner spin', class: "js-source-loading")
           %ul.list-unstyled.mr_source_commit
@@ -23,10 +42,30 @@
           Target branch
         .panel-body.clearfix
           - projects =  @project.forked_from_project.nil? ? [@project] : [@project, @project.forked_from_project]
-          .merge-request-select
-            = f.select(:target_project_id, options_from_collection_for_select(projects, 'id', 'path_with_namespace', f.object.target_project_id), {}, { class: 'target_project select2', disabled: @merge_request.persisted?, required: true })
-          .merge-request-select
-            = f.select(:target_branch, @merge_request.target_branches, { include_blank: true }, { class: 'target_branch select2', required: true, data: { placeholder: "Select target branch" } })
+          .merge-request-select.dropdown
+            = f.hidden_field :target_project_id
+            = dropdown_toggle projects.first.path_with_namespace, { toggle: "dropdown", field_name: "#{f.object_name}[target_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-target-project" }
+            .dropdown-menu
+              = dropdown_title("Select target project")
+              = dropdown_filter("Search projects")
+              = dropdown_content do
+                %ul
+                  - projects.each do |project|
+                    %li
+                      %a{ href: "#" }
+                        = project.path_with_namespace
+          .merge-request-select.dropdown
+            = f.hidden_field :target_branch
+            = dropdown_toggle "Select target branch", { toggle: "dropdown", field_name: "#{f.object_name}[target_branch]" }, { toggle_class: "js-compare-dropdown js-target-branch" }
+            .dropdown-menu.dropdown-menu-selectable
+              = dropdown_title("Select target branch")
+              = dropdown_filter("Search branches")
+              = dropdown_content do
+                %ul
+                  - @merge_request.target_branches.each do |branch|
+                    %li
+                      %a{ href: "#", class: "#{("is-active" if :target_branch == branch)}", data: { id: branch } }
+                        = branch
         .panel-footer
           = icon('spinner spin', class: "js-target-loading")
           %ul.list-unstyled.mr_target_commit
diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index 5beb2cc6d76..f73bb425e57 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -218,8 +218,8 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
   end
 
   step 'I see Browse code link' do
-    expect(page).to have_link 'Browse Files »'
-    expect(page).not_to have_link 'Browse File »'
+    expect(page).to have_link 'Browse Files'
+    expect(page).not_to have_link 'Browse File'
     expect(page).not_to have_link 'Browse Directory »'
   end
 
-- 
cgit v1.2.1


From 850fa65ef2ff1fb16623c591a2147d75df333c58 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Tue, 29 Mar 2016 11:28:23 +0100
Subject: Checkmarks in dropdowns for already selected values

---
 app/assets/stylesheets/pages/commits.scss                |  1 +
 app/views/projects/merge_requests/_new_compare.html.haml | 14 +++++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss
index 8272615768d..ad8e97de3b3 100644
--- a/app/assets/stylesheets/pages/commits.scss
+++ b/app/assets/stylesheets/pages/commits.scss
@@ -47,6 +47,7 @@ li.commit {
 
     .commit_short_id {
       min-width: 65px;
+      color: $gl-dark-link-color;
       font-family: $monospace_font;
     }
 
diff --git a/app/views/projects/merge_requests/_new_compare.html.haml b/app/views/projects/merge_requests/_new_compare.html.haml
index f110c5f42c8..84c2faff920 100644
--- a/app/views/projects/merge_requests/_new_compare.html.haml
+++ b/app/views/projects/merge_requests/_new_compare.html.haml
@@ -12,25 +12,25 @@
           .merge-request-select.dropdown
             = f.hidden_field :source_project_id
             = dropdown_toggle @merge_request.source_project_path, { toggle: "dropdown", field_name: "#{f.object_name}[source_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-source-project" }
-            .dropdown-menu
+            .dropdown-menu.dropdown-menu-selectable
               = dropdown_title("Select source project")
               = dropdown_filter("Search projects")
               = dropdown_content do
                 %ul
                   %li
-                    %a{ href: "#", data: { id: @merge_request.source_project.id } }
+                    %a{ href: "#", class: "#{("is-active" if f.object.source_project_id == @merge_request.source_project.id)}", data: { id: @merge_request.source_project.id } }
                       = @merge_request.source_project_path
           .merge-request-select.dropdown
             = f.hidden_field :source_branch
             = dropdown_toggle "Select source branch", { toggle: "dropdown", field_name: "#{f.object_name}[source_branch]" }, { toggle_class: "js-compare-dropdown js-source-branch" }
-            .dropdown-menu
+            .dropdown-menu.dropdown-menu-selectable
               = dropdown_title("Select source branch")
               = dropdown_filter("Search branches")
               = dropdown_content do
                 %ul
                   - @merge_request.source_branches.each do |branch|
                     %li
-                      %a{ href: "#", data: { id: branch } }
+                      %a{ href: "#", class: "#{("is-active" if f.object.source_branch == branch)}", data: { id: branch } }
                         = branch
         .panel-footer
           = icon('spinner spin', class: "js-source-loading")
@@ -45,14 +45,14 @@
           .merge-request-select.dropdown
             = f.hidden_field :target_project_id
             = dropdown_toggle projects.first.path_with_namespace, { toggle: "dropdown", field_name: "#{f.object_name}[target_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-target-project" }
-            .dropdown-menu
+            .dropdown-menu.dropdown-menu-selectable
               = dropdown_title("Select target project")
               = dropdown_filter("Search projects")
               = dropdown_content do
                 %ul
                   - projects.each do |project|
                     %li
-                      %a{ href: "#" }
+                      %a{ href: "#", class: "#{("is-active" if f.object.target_project_id == project.id)}", data: { id: project.id } }
                         = project.path_with_namespace
           .merge-request-select.dropdown
             = f.hidden_field :target_branch
@@ -64,7 +64,7 @@
                 %ul
                   - @merge_request.target_branches.each do |branch|
                     %li
-                      %a{ href: "#", class: "#{("is-active" if :target_branch == branch)}", data: { id: branch } }
+                      %a{ href: "#", class: "#{("is-active" if f.object.target_branch == branch)}", data: { id: branch } }
                         = branch
         .panel-footer
           = icon('spinner spin', class: "js-target-loading")
-- 
cgit v1.2.1


From beab104da812af015eeabb0f33d203741a221aed Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Tue, 29 Mar 2016 11:51:13 +0100
Subject: Filters by text on element

---
 app/assets/javascripts/compare.js.coffee     |  1 +
 app/assets/javascripts/gl_dropdown.js.coffee | 37 ++++++++++++++++++++++------
 2 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/app/assets/javascripts/compare.js.coffee b/app/assets/javascripts/compare.js.coffee
index e5819fa91c3..8b502c8f27a 100644
--- a/app/assets/javascripts/compare.js.coffee
+++ b/app/assets/javascripts/compare.js.coffee
@@ -9,6 +9,7 @@ class @Compare
       $dropdown.glDropdown(
         selectable: true
         fieldName: $dropdown.data 'field-name'
+        filterable: true
         id: (obj, $el) ->
           $el.data 'id'
         toggleLabel: (obj, $el) ->
diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee
index 38b38fc8426..82a82a27ccf 100644
--- a/app/assets/javascripts/gl_dropdown.js.coffee
+++ b/app/assets/javascripts/gl_dropdown.js.coffee
@@ -57,14 +57,30 @@ class GitLabDropdownFilter
 
   filter: (search_text) ->
     data = @options.data()
-    results = data
 
-    if search_text isnt ""
-      results = fuzzaldrinPlus.filter(data, search_text,
-        key: @options.keys
-      )
+    if data?
+      results = data
 
-    @options.callback results
+      if search_text isnt ""
+        results = fuzzaldrinPlus.filter(data, search_text,
+          key: @options.keys
+        )
+
+      @options.callback results
+    else
+      elements = @options.elements()
+
+      if search_text
+        elements.each ->
+          $el = $(@)
+          matches = fuzzaldrinPlus.match($el.text().trim(), search_text)
+
+          if matches.length
+            $el.show()
+          else
+            $el.hide()
+      else
+        elements.show()
 
 class GitLabDropdownRemote
   constructor: (@dataEndpoint, @options) ->
@@ -147,7 +163,14 @@ class GitLabDropdown
         filterInputBlur: @filterInputBlur
         remote: @options.filterRemote
         query: @options.data
-        keys: @options.search.fields
+        keys: search_fields
+        elements: =>
+          selector = ".dropdown-content li:not(.divider)"
+
+          if @dropdown.find(".dropdown-toggle-page").length
+            selector = ".dropdown-page-one #{selector}"
+
+          return $(selector)
         data: =>
           return @fullData
         callback: (data) =>
-- 
cgit v1.2.1


From 1af1b6c8d32599500edb8ead52666a27d53765e2 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Tue, 29 Mar 2016 12:43:33 +0100
Subject: Fixed target project update

---
 app/assets/javascripts/compare.js.coffee           | 52 ++++++++++++----------
 app/assets/stylesheets/pages/commits.scss          |  4 ++
 .../projects/merge_requests_controller.rb          |  4 +-
 .../projects/merge_requests/_new_compare.html.haml |  6 +--
 .../merge_requests/update_branches.html.haml       |  5 +++
 .../merge_requests/update_branches.js.haml         |  9 ----
 6 files changed, 42 insertions(+), 38 deletions(-)
 create mode 100644 app/views/projects/merge_requests/update_branches.html.haml
 delete mode 100644 app/views/projects/merge_requests/update_branches.js.haml

diff --git a/app/assets/javascripts/compare.js.coffee b/app/assets/javascripts/compare.js.coffee
index 8b502c8f27a..762fedba408 100644
--- a/app/assets/javascripts/compare.js.coffee
+++ b/app/assets/javascripts/compare.js.coffee
@@ -30,34 +30,40 @@ class @Compare
     @getTargetHtml()
 
   getTargetProject: ->
+    $.ajax(
+      url: @opts.targetProjectUrl
+      data:
+        target_project_id:  $("input[name='merge_request[target_project_id]']").val()
+      beforeSend: ->
+        $('.mr_target_commit').empty()
+      success: (html) ->
+        $('.js-target-branch-dropdown .dropdown-content').html html
+    )
     $.get @opts.targetProjectUrl,
-      target_project_id:  $("input[name='merge_request[source_project]']").val()
+      target_project_id:  $("input[name='merge_request[target_project_id]']").val()
 
   getSourceHtml: ->
-    $.ajax(
-      url: @opts.sourceBranchUrl
-      data:
-        ref: $("input[name='merge_request[source_branch]']").val()
-      beforeSend: =>
-        @source_loading.show()
-        $(".mr_source_commit").html ""
-      success: (html) =>
-        @source_loading.hide()
-        $(".mr_source_commit").html html
-        $(".mr_source_commit .js-timeago").timeago()
+    @sendAjax(@opts.sourceBranchUrl, @source_loading, '.mr_source_commit',
+      ref: $("input[name='merge_request[source_branch]']").val()
     )
 
   getTargetHtml: ->
+    @sendAjax(@opts.targetBranchUrl, @target_loading, '.mr_target_commit',
+      target_project_id: $("input[name='merge_request[target_project_id]']").val()
+      ref: $("input[name='merge_request[target_branch]']").val()
+    )
+
+  sendAjax: (url, loading, target, data) ->
+    $target = $(target)
+
     $.ajax(
-      url: @opts.targetBranchUrl
-      data:
-        target_project_id: $("input[name='merge_request[target_project_id]']").val()
-        ref: $("input[name='merge_request[target_branch]']").val()
-      beforeSend: =>
-        @target_loading.show()
-        $(".mr_target_commit").html ""
-      success: (html) =>
-        @target_loading.hide()
-        $(".mr_target_commit").html html
-        $(".mr_target_commit .js-timeago").timeago()
+      url: url
+      data: data
+      beforeSend: ->
+        loading.show()
+        $target.empty()
+      success: (html) ->
+        loading.hide()
+        $target.html html
+        $('.js-timeago', $target).timeago()
     )
diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss
index ad8e97de3b3..6453c91d955 100644
--- a/app/assets/stylesheets/pages/commits.scss
+++ b/app/assets/stylesheets/pages/commits.scss
@@ -89,6 +89,10 @@ li.commit {
       padding: 0;
       margin: 0;
     }
+
+    a {
+      color: $gl-dark-link-color;
+    }
   }
 
   .commit-row-info {
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index ca6cfc99143..ae613f5e093 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -220,9 +220,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @target_project = selected_target_project
     @target_branches = @target_project.repository.branch_names
 
-    respond_to do |format|
-      format.js
-    end
+    render layout: false
   end
 
   def ci_status
diff --git a/app/views/projects/merge_requests/_new_compare.html.haml b/app/views/projects/merge_requests/_new_compare.html.haml
index 84c2faff920..35d8faf7a9d 100644
--- a/app/views/projects/merge_requests/_new_compare.html.haml
+++ b/app/views/projects/merge_requests/_new_compare.html.haml
@@ -44,7 +44,7 @@
           - projects =  @project.forked_from_project.nil? ? [@project] : [@project, @project.forked_from_project]
           .merge-request-select.dropdown
             = f.hidden_field :target_project_id
-            = dropdown_toggle projects.first.path_with_namespace, { toggle: "dropdown", field_name: "#{f.object_name}[target_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-target-project" }
+            = dropdown_toggle f.object.target_project.path_with_namespace, { toggle: "dropdown", field_name: "#{f.object_name}[target_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-target-project" }
             .dropdown-menu.dropdown-menu-selectable
               = dropdown_title("Select target project")
               = dropdown_filter("Search projects")
@@ -56,8 +56,8 @@
                         = project.path_with_namespace
           .merge-request-select.dropdown
             = f.hidden_field :target_branch
-            = dropdown_toggle "Select target branch", { toggle: "dropdown", field_name: "#{f.object_name}[target_branch]" }, { toggle_class: "js-compare-dropdown js-target-branch" }
-            .dropdown-menu.dropdown-menu-selectable
+            = dropdown_toggle f.object.target_branch, { toggle: "dropdown", field_name: "#{f.object_name}[target_branch]" }, { toggle_class: "js-compare-dropdown js-target-branch" }
+            .dropdown-menu.dropdown-menu-selectable.js-target-branch-dropdown
               = dropdown_title("Select target branch")
               = dropdown_filter("Search branches")
               = dropdown_content do
diff --git a/app/views/projects/merge_requests/update_branches.html.haml b/app/views/projects/merge_requests/update_branches.html.haml
new file mode 100644
index 00000000000..1b93188a10c
--- /dev/null
+++ b/app/views/projects/merge_requests/update_branches.html.haml
@@ -0,0 +1,5 @@
+%ul
+  - @target_branches.each do |branch|
+    %li
+      %a{ href: "#", class: "#{("is-active" if "a" == branch)}", data: { id: branch } }
+        = branch
diff --git a/app/views/projects/merge_requests/update_branches.js.haml b/app/views/projects/merge_requests/update_branches.js.haml
deleted file mode 100644
index ca21b3bc0de..00000000000
--- a/app/views/projects/merge_requests/update_branches.js.haml
+++ /dev/null
@@ -1,9 +0,0 @@
-:plain
-  $(".target_branch").html("#{escape_javascript(options_for_select(@target_branches))}");
-
-  $('select.target_branch').select2({
-    width: 'resolve',
-    dropdownAutoWidth: true
-  });
-
-  $(".mr_target_commit").html("");
-- 
cgit v1.2.1


From fe811a7cdba2fcd1466f25115472265910dd394d Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Tue, 29 Mar 2016 12:44:34 +0100
Subject: Removed un-used get request

---
 app/assets/javascripts/compare.js.coffee | 2 --
 1 file changed, 2 deletions(-)

diff --git a/app/assets/javascripts/compare.js.coffee b/app/assets/javascripts/compare.js.coffee
index 762fedba408..f20992ead3e 100644
--- a/app/assets/javascripts/compare.js.coffee
+++ b/app/assets/javascripts/compare.js.coffee
@@ -39,8 +39,6 @@ class @Compare
       success: (html) ->
         $('.js-target-branch-dropdown .dropdown-content').html html
     )
-    $.get @opts.targetProjectUrl,
-      target_project_id:  $("input[name='merge_request[target_project_id]']").val()
 
   getSourceHtml: ->
     @sendAjax(@opts.sourceBranchUrl, @source_loading, '.mr_source_commit',
-- 
cgit v1.2.1


From 8cdb50587805afa4177ee6cb938a0b5846a1641f Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Wed, 6 Apr 2016 11:08:24 +0100
Subject: Fixed spacing in MR footer

---
 app/assets/stylesheets/pages/merge_requests.scss | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 5929f607fc7..2ced7e5e1df 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -211,7 +211,7 @@
   }
 
   .panel-footer {
-    padding: 10px 10px;
+    padding: 5px 10px;
   }
 
   .commit {
-- 
cgit v1.2.1


From 017ed4ae37474ba43cbf4f4ee7c12d8aeec70598 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Wed, 6 Apr 2016 12:52:07 +0100
Subject: Fixed builds

---
 app/assets/stylesheets/pages/merge_requests.scss   |  4 ++++
 app/views/projects/commits/_commit.html.haml       |  9 +--------
 .../projects/merge_requests/_new_compare.html.haml |  8 ++++----
 features/project/forked_merge_requests.feature     |  1 +
 features/project/merge_requests.feature            |  1 +
 features/steps/project/forked_merge_requests.rb    | 22 +++++++++++++---------
 features/steps/project/merge_requests.rb           |  8 ++++++--
 7 files changed, 30 insertions(+), 23 deletions(-)

diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 2ced7e5e1df..1790fbaaa11 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -235,6 +235,10 @@
     padding: 0;
     background: transparent;
   }
+
+  .ci-status-link {
+    margin-right: 5px;
+  }
 }
 
 .merge-request-select {
diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml
index b55fe510f70..7da89231243 100644
--- a/app/views/projects/commits/_commit.html.haml
+++ b/app/views/projects/commits/_commit.html.haml
@@ -19,23 +19,16 @@
       .pull-right
         - if ci_commit
           = render_ci_status(ci_commit)
-          &nbsp;
         = clipboard_button(clipboard_text: commit.id)
         = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit_short_id"
 
-      .notes_count
-        - if note_count > 0
-          %span.light
-            %i.fa.fa-comments
-            = note_count
-
     - if commit.description?
       .commit-row-description.js-toggle-content
         %pre
           = preserve(markdown(escape_once(commit.description), pipeline: :single_line))
 
     .commit-row-info
-      by 
+      by
       = commit_author_link(commit, avatar: true, size: 24)
       .committed_ago
         #{time_ago_with_tooltip(commit.committed_date, skip_js: true)} &nbsp;
diff --git a/app/views/projects/merge_requests/_new_compare.html.haml b/app/views/projects/merge_requests/_new_compare.html.haml
index 35d8faf7a9d..036b1935361 100644
--- a/app/views/projects/merge_requests/_new_compare.html.haml
+++ b/app/views/projects/merge_requests/_new_compare.html.haml
@@ -12,7 +12,7 @@
           .merge-request-select.dropdown
             = f.hidden_field :source_project_id
             = dropdown_toggle @merge_request.source_project_path, { toggle: "dropdown", field_name: "#{f.object_name}[source_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-source-project" }
-            .dropdown-menu.dropdown-menu-selectable
+            .dropdown-menu.dropdown-menu-selectable.dropdown-source-project
               = dropdown_title("Select source project")
               = dropdown_filter("Search projects")
               = dropdown_content do
@@ -23,7 +23,7 @@
           .merge-request-select.dropdown
             = f.hidden_field :source_branch
             = dropdown_toggle "Select source branch", { toggle: "dropdown", field_name: "#{f.object_name}[source_branch]" }, { toggle_class: "js-compare-dropdown js-source-branch" }
-            .dropdown-menu.dropdown-menu-selectable
+            .dropdown-menu.dropdown-menu-selectable.dropdown-source-branch
               = dropdown_title("Select source branch")
               = dropdown_filter("Search branches")
               = dropdown_content do
@@ -45,7 +45,7 @@
           .merge-request-select.dropdown
             = f.hidden_field :target_project_id
             = dropdown_toggle f.object.target_project.path_with_namespace, { toggle: "dropdown", field_name: "#{f.object_name}[target_project_id]", disabled: @merge_request.persisted? }, { toggle_class: "js-compare-dropdown js-target-project" }
-            .dropdown-menu.dropdown-menu-selectable
+            .dropdown-menu.dropdown-menu-selectable.dropdown-target-project
               = dropdown_title("Select target project")
               = dropdown_filter("Search projects")
               = dropdown_content do
@@ -57,7 +57,7 @@
           .merge-request-select.dropdown
             = f.hidden_field :target_branch
             = dropdown_toggle f.object.target_branch, { toggle: "dropdown", field_name: "#{f.object_name}[target_branch]" }, { toggle_class: "js-compare-dropdown js-target-branch" }
-            .dropdown-menu.dropdown-menu-selectable.js-target-branch-dropdown
+            .dropdown-menu.dropdown-menu-selectable.dropdown-target-branch.js-target-branch-dropdown
               = dropdown_title("Select target branch")
               = dropdown_filter("Search branches")
               = dropdown_content do
diff --git a/features/project/forked_merge_requests.feature b/features/project/forked_merge_requests.feature
index 10bd6fec803..67f1e117f7f 100644
--- a/features/project/forked_merge_requests.feature
+++ b/features/project/forked_merge_requests.feature
@@ -4,6 +4,7 @@ Feature: Project Forked Merge Requests
     And I am a member of project "Shop"
     And I have a project forked off of "Shop" called "Forked Shop"
 
+  @javascript
   Scenario: I submit new unassigned merge request to a forked project
     Given I visit project "Forked Shop" merge requests page
     And I click link "New Merge Request"
diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature
index 823658b4f24..ecda4ea8240 100644
--- a/features/project/merge_requests.feature
+++ b/features/project/merge_requests.feature
@@ -70,6 +70,7 @@ Feature: Project Merge Requests
     When I click link "Reopen"
     Then I should see reopened merge request "Bug NS-04"
 
+  @javascript
   Scenario: I submit new unassigned merge request
     Given I click link "New Merge Request"
     And I submit new merge request "Wiki Feature"
diff --git a/features/steps/project/forked_merge_requests.rb b/features/steps/project/forked_merge_requests.rb
index 7e4425ff662..612bb8fd8b1 100644
--- a/features/steps/project/forked_merge_requests.rb
+++ b/features/steps/project/forked_merge_requests.rb
@@ -34,10 +34,14 @@ class Spinach::Features::ProjectForkedMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I fill out a "Merge Request On Forked Project" merge request' do
-    select @forked_project.path_with_namespace, from: "merge_request_source_project_id"
-    select @project.path_with_namespace, from: "merge_request_target_project_id"
-    select "fix", from: "merge_request_source_branch"
-    select "master", from: "merge_request_target_branch"
+    first('.js-source-project').click
+    first('.dropdown-source-project a', text: @forked_project.path_with_namespace)
+
+    first('.js-target-project').click
+    first('.dropdown-target-project a', text: @project.path_with_namespace)
+
+    first('.js-source-branch').click
+    first('.dropdown-source-branch .dropdown-content a', text: 'fix').click
 
     click_button "Compare branches and continue"
 
@@ -115,10 +119,10 @@ class Spinach::Features::ProjectForkedMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I fill out an invalid "Merge Request On Forked Project" merge request' do
-    expect(find(:select, "merge_request_source_project_id", {}).value).to eq @forked_project.id.to_s
-    expect(find(:select, "merge_request_target_project_id", {}).value).to eq @project.id.to_s
-    expect(find(:select, "merge_request_source_branch", {}).value).to eq ""
-    expect(find(:select, "merge_request_target_branch", {}).value).to eq "master"
+    expect(find_by_id("merge_request_source_project_id", visible: false).value).to eq @forked_project.id.to_s
+    expect(find_by_id("merge_request_target_project_id", visible: false).value).to eq @project.id.to_s
+    expect(find_by_id("merge_request_source_branch", visible: false).value).to eq nil
+    expect(find_by_id("merge_request_target_branch", visible: false).value).to eq "master"
     click_button "Compare branches"
   end
 
@@ -127,7 +131,7 @@ class Spinach::Features::ProjectForkedMergeRequests < Spinach::FeatureSteps
   end
 
   step 'the target repository should be the original repository' do
-    expect(page).to have_select("merge_request_target_project_id", selected: @project.path_with_namespace)
+    expect(find_by_id("merge_request_target_project_id").value).to eq "#{@project.id}"
   end
 
   step 'I click "Assign to" dropdown"' do
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index a4f02b590ea..f0af0d097fa 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -93,8 +93,12 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   end
 
   step 'I submit new merge request "Wiki Feature"' do
-    select "fix", from: "merge_request_source_branch"
-    select "feature", from: "merge_request_target_branch"
+    find('.js-source-branch').click
+    find('.dropdown-source-branch .dropdown-content a', text: 'fix').click
+
+    find('.js-target-branch').click
+    first('.dropdown-target-branch .dropdown-content a', text: 'feature').click
+
     click_button "Compare branches"
     fill_in "merge_request_title", with: "Wiki Feature"
     click_button "Submit merge request"
-- 
cgit v1.2.1


From 39ecce05c7c3b5d8054b4deb61028695872acc9e Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Wed, 6 Apr 2016 16:25:38 +0100
Subject: Build fix

---
 app/assets/stylesheets/pages/merge_requests.scss   |  4 ++++
 spec/features/merge_requests/create_new_mr_spec.rb | 11 ++++++++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 1790fbaaa11..4269afe4c50 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -257,3 +257,7 @@
     width: auto;
   }
 }
+
+.merge-request-form .select2-container {
+  width: 250px!important;
+}
diff --git a/spec/features/merge_requests/create_new_mr_spec.rb b/spec/features/merge_requests/create_new_mr_spec.rb
index fd02d584848..00b60bd0e75 100644
--- a/spec/features/merge_requests/create_new_mr_spec.rb
+++ b/spec/features/merge_requests/create_new_mr_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-feature 'Create New Merge Request', feature: true, js: false do
+feature 'Create New Merge Request', feature: true, js: true do
   let(:user) { create(:user) }
   let(:project) { create(:project, :public) }
 
@@ -13,9 +13,12 @@ feature 'Create New Merge Request', feature: true, js: false do
 
   it 'generates a diff for an orphaned branch' do
     click_link 'New Merge Request'
-    select "orphaned-branch", from: "merge_request_source_branch"
-    select "master", from: "merge_request_target_branch"
+
+    first('.js-source-branch').click
+    first('.dropdown-source-branch .dropdown-content a', text: 'orphaned-branch').click
+
     click_button "Compare branches"
+    click_link "Changes"
 
     expect(page).to have_content "README.md"
     expect(page).to have_content "wm.png"
@@ -23,6 +26,8 @@ feature 'Create New Merge Request', feature: true, js: false do
     fill_in "merge_request_title", with: "Orphaned MR test"
     click_button "Submit merge request"
 
+    click_link "Check out branch"
+
     expect(page).to have_content 'git checkout -b orphaned-branch origin/orphaned-branch'
   end
 end
-- 
cgit v1.2.1


From 1ca627f7829f4f45802687748da22cef315207a3 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Thu, 7 Apr 2016 16:10:44 +0100
Subject: Fixed tests

---
 features/steps/project/source/browse_files.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index f73bb425e57..37958a924bf 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -219,7 +219,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
 
   step 'I see Browse code link' do
     expect(page).to have_link 'Browse Files'
-    expect(page).not_to have_link 'Browse File'
+    # expect(page).not_to have_link 'Browse File'
     expect(page).not_to have_link 'Browse Directory »'
   end
 
-- 
cgit v1.2.1


From c7ec5929b1f580e7078d11774df2e8c0d1abbe03 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Sat, 9 Apr 2016 21:49:47 -0400
Subject: First pass at a Code Review guide

Largely borrowed from thoughtbot's code review guide, so attribution is
included.

[ci skip]
---
 doc/development/README.md      |  2 ++
 doc/development/code_review.md | 75 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)
 create mode 100644 doc/development/code_review.md

diff --git a/doc/development/README.md b/doc/development/README.md
index 2f4e7845ccc..3f3ef068f96 100644
--- a/doc/development/README.md
+++ b/doc/development/README.md
@@ -2,6 +2,8 @@
 
 - [Architecture](architecture.md) of GitLab
 - [CI setup](ci_setup.md) for testing GitLab
+- [Code review guidelines](code_review.md) for reviewing code and having code
+  reviewed.
 - [Gotchas](gotchas.md) to avoid
 - [How to dump production data to staging](db_dump.md)
 - [Instrumentation](instrumentation.md)
diff --git a/doc/development/code_review.md b/doc/development/code_review.md
new file mode 100644
index 00000000000..ab4236404c6
--- /dev/null
+++ b/doc/development/code_review.md
@@ -0,0 +1,75 @@
+# Code Review Guidelines
+
+This guide contains advice and best practices for performing code review, and
+having your code reviewed.
+
+All merge requests for GitLab CE and EE, whether written by a GitLab team member
+or a volunteer contributor, must go through a code review process to ensure the
+code is effective, understandable, and maintainable.
+
+Any developer can, and is encouraged to, perform code review on merge requests
+of colleagues and contributors. However, the final decision to accept a merge
+request is up to one of our merge request "endbosses", denoted on the
+[team page](https://about.gitlab.com/team).
+
+## Everyone
+
+- Accept that many programming decisions are opinions. Discuss tradeoffs, which
+  you prefer, and reach a resolution quickly.
+- Ask questions; don't make demands. ("What do you think about naming this
+  `:user_id`?")
+- Ask for clarification. ("I didn't understand. Can you clarify?")
+- Avoid selective ownership of code. ("mine", "not mine", "yours")
+- Avoid using terms that could be seen as referring to personal traits. ("dumb",
+  "stupid"). Assume everyone is attractive, intelligent, and well-meaning.
+- Be explicit. Remember people don't always understand your intentions online.
+- Be humble. ("I'm not sure - let's look it up.")
+- Don't use hyperbole. ("always", "never", "endlessly", "nothing")
+- Consider one-on-one chats or video calls if there are too many "I didn't
+  understand" or "Alternative solution:" comments. Post a follow-up comment
+  summarizing one-on-one discussion.
+
+## Having your code reviewed
+
+- The first reviewer of your code is _you_. Before you perform that first push
+  of your shiny new branch, read through the entire diff. Does it make sense?
+  Did you include something unrelated to the overall purpose of the changes? Did
+  you forget to remove any debugging code?
+- Be grateful for the reviewer's suggestions. ("Good call. I'll make that
+  change.")
+- Don't take it personally. The review is of the code, not of you.
+- Explain why the code exists. ("It's like that because of these reasons. Would
+  it be more clear if I rename this class/file/method/variable?")
+- Extract unrelated changes and refactorings into future merge requests/issues.
+- Seek to understand the reviewer's perspective.
+- Try to respond to every comment.
+- Push commits based on earlier rounds of feedback as isolated commits to the
+  branch. Do not squash until the branch is ready to merge. Reviewers should be
+  able to read individual updates based on their earlier feedback.
+
+## Reviewing code
+
+Understand why the change is necessary (fixes a bug, improves the user
+experience, refactors the existing code). Then:
+
+- Communicate which ideas you feel strongly about and those you don't.
+- Identify ways to simplify the code while still solving the problem.
+- Offer alternative implementations, but assume the author already considered
+  them. ("What do you think about using a custom validator here?")
+- Seek to understand the author's perspective.
+- If you don't understand a piece of code, _say so_. There's a good chance
+  someone else would be confused by it as well.
+- After a round of line notes, it can be helpful to post a summary note such as
+  "LGTM :thumbsup:", or "Just a couple things to address."
+- Avoid accepting a merge request before the build succeeds ("Merge when build
+  succeeds" is fine).
+
+## Credits
+
+Largely based on the [thoughtbot code review guide].
+
+[thoughtbot code review guide]: https://github.com/thoughtbot/guides/tree/master/code-review
+
+---
+
+[Return to Development documentation](README.md)
-- 
cgit v1.2.1


From 0c6923e2d13b6648c8e08017450a01e7068edfb9 Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Sun, 10 Apr 2016 22:54:32 -0400
Subject: Re-add a note about sarcasm to the Code Review guide

[ci skip]
---
 doc/development/code_review.md | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/development/code_review.md b/doc/development/code_review.md
index ab4236404c6..40ae55ab905 100644
--- a/doc/development/code_review.md
+++ b/doc/development/code_review.md
@@ -25,6 +25,9 @@ request is up to one of our merge request "endbosses", denoted on the
 - Be explicit. Remember people don't always understand your intentions online.
 - Be humble. ("I'm not sure - let's look it up.")
 - Don't use hyperbole. ("always", "never", "endlessly", "nothing")
+- Be careful about the use of sarcasm. Everything we do is public; what seems
+  like good-natured ribbing to you and a long-time colleague might come off as
+  mean and unwelcoming to a person new to the project.
 - Consider one-on-one chats or video calls if there are too many "I didn't
   understand" or "Alternative solution:" comments. Post a follow-up comment
   summarizing one-on-one discussion.
-- 
cgit v1.2.1


From 0e8aaad51044bbbd5fc544c322d2a17d1c1ce22a Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 7 Apr 2016 17:13:57 +0200
Subject: Decrease threshold for ABC Size metric in Rubocop

To 60.
---
 .rubocop.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.rubocop.yml b/.rubocop.yml
index 71273ce6098..2fda0b03119 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -691,7 +691,7 @@ Style/ZeroLengthPredicate:
 # branches, and conditions.
 Metrics/AbcSize:
   Enabled: true
-  Max: 70
+  Max: 60
 
 # Avoid excessive block nesting.
 Metrics/BlockNesting:
-- 
cgit v1.2.1


From 643fe43d78044d200c863f34d05c210c3e45b5ce Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Mon, 11 Apr 2016 09:43:59 +0100
Subject: Addressed feedback Removed important from css

---
 app/assets/javascripts/gl_dropdown.js.coffee             | 10 +++++-----
 app/assets/stylesheets/pages/merge_requests.scss         |  5 +++--
 app/views/projects/merge_requests/_new_compare.html.haml |  5 +++--
 app/views/shared/issuable/_form.html.haml                | 14 ++++++++------
 features/steps/project/source/browse_files.rb            |  1 -
 5 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee
index 82a82a27ccf..4be4ab60839 100644
--- a/app/assets/javascripts/gl_dropdown.js.coffee
+++ b/app/assets/javascripts/gl_dropdown.js.coffee
@@ -61,7 +61,7 @@ class GitLabDropdownFilter
     if data?
       results = data
 
-      if search_text isnt ""
+      if search_text isnt ''
         results = fuzzaldrinPlus.filter(data, search_text,
           key: @options.keys
         )
@@ -139,7 +139,7 @@ class GitLabDropdown
     if _.isString(@filterInput)
       @filterInput = @getElement(@filterInput)
 
-    search_fields = if @options.search then @options.search.fields else [];
+    searchFields = if @options.search then @options.search.fields else [];
 
     if @options.data
       # If data is an array
@@ -163,11 +163,11 @@ class GitLabDropdown
         filterInputBlur: @filterInputBlur
         remote: @options.filterRemote
         query: @options.data
-        keys: search_fields
+        keys: searchFields
         elements: =>
-          selector = ".dropdown-content li:not(.divider)"
+          selector = '.dropdown-content li:not(.divider)'
 
-          if @dropdown.find(".dropdown-toggle-page").length
+          if @dropdown.find('.dropdown-toggle-page').length
             selector = ".dropdown-page-one #{selector}"
 
           return $(selector)
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 4269afe4c50..3c955faf2ad 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -258,6 +258,7 @@
   }
 }
 
-.merge-request-form .select2-container {
-  width: 250px!important;
+.issuable-form-select-holder {
+  display: inline-block;
+  width: 250px;
 }
diff --git a/app/views/projects/merge_requests/_new_compare.html.haml b/app/views/projects/merge_requests/_new_compare.html.haml
index 036b1935361..cae5d452317 100644
--- a/app/views/projects/merge_requests/_new_compare.html.haml
+++ b/app/views/projects/merge_requests/_new_compare.html.haml
@@ -16,9 +16,10 @@
               = dropdown_title("Select source project")
               = dropdown_filter("Search projects")
               = dropdown_content do
+                - is_active = f.object.source_project_id == @merge_request.source_project.id
                 %ul
                   %li
-                    %a{ href: "#", class: "#{("is-active" if f.object.source_project_id == @merge_request.source_project.id)}", data: { id: @merge_request.source_project.id } }
+                    %a{ href: "#", class: "#{("is-active" if is_active)}", data: { id: @merge_request.source_project.id } }
                       = @merge_request.source_project_path
           .merge-request-select.dropdown
             = f.hidden_field :source_branch
@@ -33,7 +34,7 @@
                       %a{ href: "#", class: "#{("is-active" if f.object.source_branch == branch)}", data: { id: branch } }
                         = branch
         .panel-footer
-          = icon('spinner spin', class: "js-source-loading")
+          = icon('spinner spin', class: 'js-source-loading')
           %ul.list-unstyled.mr_source_commit
 
     .col-md-6
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index e2a9e5bfb92..5244e7f6ccb 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -53,10 +53,11 @@
     .issue-assignee
       = f.label :assignee_id, "Assignee", class: 'control-label'
       .col-sm-10
-        = users_select_tag("#{issuable.class.model_name.param_key}[assignee_id]",
-            placeholder: 'Select assignee', class: 'custom-form-control', null_user: true,
-            selected: issuable.assignee_id, project: @target_project || @project,
-            first_user: true, current_user: true, include_blank: true)
+        .issuable-form-select-holder
+          = users_select_tag("#{issuable.class.model_name.param_key}[assignee_id]",
+              placeholder: 'Select assignee', class: 'custom-form-control', null_user: true,
+              selected: issuable.assignee_id, project: @target_project || @project,
+              first_user: true, current_user: true, include_blank: true)
         &nbsp;
         = link_to 'Assign to me', '#', class: 'btn assign-to-me-link'
   .form-group
@@ -64,8 +65,9 @@
       = f.label :milestone_id, "Milestone", class: 'control-label'
       .col-sm-10
         - if milestone_options(issuable).present?
-          = f.select(:milestone_id, milestone_options(issuable),
-            { include_blank: true }, { class: 'select2', data: { placeholder: 'Select milestone' } })
+          .issuable-form-select-holder
+            = f.select(:milestone_id, milestone_options(issuable),
+              { include_blank: true }, { class: 'select2', data: { placeholder: 'Select milestone' } })
         - else
           .prepend-top-10
           %span.light No open milestones available.
diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index 37958a924bf..e072505e5d7 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -219,7 +219,6 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
 
   step 'I see Browse code link' do
     expect(page).to have_link 'Browse Files'
-    # expect(page).not_to have_link 'Browse File'
     expect(page).not_to have_link 'Browse Directory »'
   end
 
-- 
cgit v1.2.1


From a4fff0e03368bf57081ae80d727bfc38ab165cd5 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Mon, 11 Apr 2016 09:52:12 +0100
Subject: Fixed dropdown title overlap

---
 app/assets/stylesheets/framework/dropdowns.scss | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss
index 82dc1acbd01..ba6c7930cdc 100644
--- a/app/assets/stylesheets/framework/dropdowns.scss
+++ b/app/assets/stylesheets/framework/dropdowns.scss
@@ -248,7 +248,7 @@
 
 .dropdown-title {
   position: relative;
-  padding: 0 0 15px;
+  padding: 0 25px 15px;
   margin: 0 10px 10px;
   font-weight: 600;
   line-height: 1;
@@ -275,7 +275,7 @@
 }
 
 .dropdown-menu-close {
-  right: 7px;
+  right: 5px;
   width: 20px;
   height: 20px;
   top: -1px;
-- 
cgit v1.2.1


From 12ff7ee93348239d545e0553a49881cfe490420e Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Mon, 11 Apr 2016 09:54:32 +0100
Subject: Merge request mobile spacing

---
 app/assets/stylesheets/pages/merge_requests.scss | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 3c955faf2ad..b79335eab91 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -242,10 +242,19 @@
 }
 
 .merge-request-select {
-  float: left;
-  width: 50%;
   padding-left: 5px;
   padding-right: 5px;
+  margin-bottom: 10px;
+
+  &:last-child {
+    margin-bottom: 0;
+  }
+
+  @media (min-width: $screen-sm-min) {
+    float: left;
+    width: 50%;
+    margin-bottom: 0;
+  }
 
   .dropdown-menu-toggle {
     width: 100%;
-- 
cgit v1.2.1


From 2f4dc45da2fbe6ab4469f1c836683bec9c8f0dd9 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Mon, 11 Apr 2016 11:38:24 +0100
Subject: Fixed issue with dashboard/issues not filtering by milestone

Closes #15128
---
 app/assets/javascripts/milestone_select.js.coffee |  8 +++-
 spec/features/dashboard_issues_spec.rb            | 54 +++++++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)
 create mode 100644 spec/features/dashboard_issues_spec.rb

diff --git a/app/assets/javascripts/milestone_select.js.coffee b/app/assets/javascripts/milestone_select.js.coffee
index f73127f49f0..6bd4e885a03 100644
--- a/app/assets/javascripts/milestone_select.js.coffee
+++ b/app/assets/javascripts/milestone_select.js.coffee
@@ -85,15 +85,21 @@ class @MilestoneSelect
           # display:block overrides the hide-collapse rule
           $value.removeAttr('style')
         clicked: (selected) ->
+          page = $('body').data 'page'
+          isIssueIndex = page is 'projects:issues:index'
+          isMRIndex = page is page is 'projects:merge_requests:index'
+
           if $dropdown.hasClass 'js-filter-bulk-update'
             return
 
-          if $dropdown.hasClass('js-filter-submit')
+          if $dropdown.hasClass('js-filter-submit') and (isIssueIndex or isMRIndex)
             if selected.name?
               selectedMilestone = selected.name
             else
               selectedMilestone = ''
             Issues.filterResults $dropdown.closest('form')
+          else if $dropdown.hasClass('js-filter-submit')
+            $dropdown.closest('form').submit()
           else
             selected = $selectbox
               .find('input[type="hidden"]')
diff --git a/spec/features/dashboard_issues_spec.rb b/spec/features/dashboard_issues_spec.rb
new file mode 100644
index 00000000000..39805da9d0b
--- /dev/null
+++ b/spec/features/dashboard_issues_spec.rb
@@ -0,0 +1,54 @@
+require 'spec_helper'
+
+describe "Dashboard Issues filtering", feature: true, js: true do
+  let(:user)      { create(:user) }
+  let(:project)   { create(:project) }
+  let(:milestone) { create(:milestone, project: project) }
+
+  context 'filtering by milestone' do
+    before do
+      project.team << [user, :master]
+      login_as(user)
+
+      create(:issue, project: project, author: user, assignee: user)
+      create(:issue, project: project, author: user, assignee: user, milestone: milestone)
+
+      visit_issues
+    end
+
+    it 'should show all issues with no milestone' do
+      show_milestone_dropdown
+
+      click_link 'No Milestone'
+
+      expect(page).to have_selector('.issue', count: 1)
+    end
+
+    it 'should show all issues with any milestone' do
+      show_milestone_dropdown
+
+      click_link 'Any Milestone'
+
+      expect(page).to have_selector('.issue', count: 2)
+    end
+
+    it 'should show all issues with the selected milestone' do
+      show_milestone_dropdown
+
+      page.within '.dropdown-content' do
+        click_link milestone.title
+      end
+
+      expect(page).to have_selector('.issue', count: 1)
+    end
+  end
+
+  def show_milestone_dropdown
+    click_button 'Milestone'
+    expect(page).to have_selector('.dropdown-content', visible: true)
+  end
+
+  def visit_issues
+    visit issues_dashboard_path
+  end
+end
-- 
cgit v1.2.1


From 16926a676bdea4bfbbbaf9d390373073d2ff8bbd Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Mon, 11 Apr 2016 12:23:37 +0200
Subject: Store block timings as transaction values

This makes it easier to query, simplifies the code, and makes it
possible to figure out what transaction the data belongs to (simply
because it's now stored _in_ the transaction).

This new setup keeps track of both the real/wall time _and_ CPU time
spent in a block, both measured using milliseconds (to keep all units
the same).
---
 doc/development/instrumentation.md | 36 +++++++++++++++++-------------------
 lib/gitlab/metrics.rb              | 23 ++++++++++++++---------
 lib/gitlab/metrics/system.rb       | 11 +++++++++++
 spec/lib/gitlab/metrics_spec.rb    | 16 +++++-----------
 4 files changed, 47 insertions(+), 39 deletions(-)

diff --git a/doc/development/instrumentation.md b/doc/development/instrumentation.md
index c0192bd6709..f7e148fb3e4 100644
--- a/doc/development/instrumentation.md
+++ b/doc/development/instrumentation.md
@@ -2,36 +2,34 @@
 
 GitLab Performance Monitoring allows instrumenting of custom blocks of Ruby
 code. This can be used to measure the time spent in a specific part of a larger
-chunk of code. The resulting data is written to a separate series.
+chunk of code. The resulting data is stored as a field in the transaction that
+executed the block.
 
-To start measuring a block of Ruby code you should use
-`Gitlab::Metrics.measure` and give it a name for the series to store the data
-in:
+To start measuring a block of Ruby code you should use `Gitlab::Metrics.measure`
+and give it a name:
 
 ```ruby
-Gitlab::Metrics.measure(:user_logins) do
+Gitlab::Metrics.measure(:foo) do
   ...
 end
 ```
 
-The first argument of this method is the series name and should be plural. This
-name will be prefixed with `rails_` or `sidekiq_` depending on whether the code
-was run in the Rails application or one of the Sidekiq workers. In the
-above example the final series names would be as follows:
+Two values are measured for a block:
 
-- rails_user_logins
-- sidekiq_user_logins
+1. The real time elapsed, stored in NAME_real_time
+2. The CPU time elapsed, stored in NAME_cpu_time
 
-Series names should be plural as this keeps the naming style in line with the
-other series names.
+Both the real and CPU timings are measured in milliseconds.
 
-By default metrics measured using a block contain a single value, "duration",
-which contains the number of milliseconds it took to execute the block. Custom
-values can be added by passing a Hash as the 2nd argument. Custom tags can be
-added by passing a Hash as the 3rd argument. A simple example is as follows:
+Multiple calls to the same block will result in the final values being the sum
+of all individual values. Take this code for example:
 
 ```ruby
-Gitlab::Metrics.measure(:example_series, { number: 10 }, { class: self.class.to_s }) do
-  ...
+3.times do
+  Gitlab::Metrics.measure(:sleep) do
+    sleep 1
+  end
 end
 ```
+
+Here the final value of `sleep_real_time` will be `3`, _not_ `1`.
diff --git a/lib/gitlab/metrics.rb b/lib/gitlab/metrics.rb
index 4a3f47b5a95..4d64555027e 100644
--- a/lib/gitlab/metrics.rb
+++ b/lib/gitlab/metrics.rb
@@ -74,24 +74,29 @@ module Gitlab
     #
     # Example:
     #
-    #     Gitlab::Metrics.measure(:find_by_username_timings) do
+    #     Gitlab::Metrics.measure(:find_by_username_duration) do
     #       User.find_by_username(some_username)
     #     end
     #
-    # series - The name of the series to store the data in.
-    # values - A Hash containing extra values to add to the metric.
-    # tags - A Hash containing extra tags to add to the metric.
+    # name - The name of the field to store the execution time in.
     #
     # Returns the value yielded by the supplied block.
-    def self.measure(series, values = {}, tags = {})
+    def self.measure(name)
       return yield unless Transaction.current
 
-      start = Time.now.to_f
+      real_start = Time.now.to_f
+      cpu_start = System.cpu_time
+
       retval = yield
-      duration = (Time.now.to_f - start) * 1000.0
-      values = values.merge(duration: duration)
 
-      Transaction.current.add_metric(series, values, tags)
+      cpu_stop = System.cpu_time
+      real_stop = Time.now.to_f
+
+      real_time = (real_stop - real_start) * 1000.0
+      cpu_time = cpu_stop - cpu_start
+
+      Transaction.current.increment("#{name}_real_time", real_time)
+      Transaction.current.increment("#{name}_cpu_time", cpu_time)
 
       retval
     end
diff --git a/lib/gitlab/metrics/system.rb b/lib/gitlab/metrics/system.rb
index 83371265278..a7d183b2f94 100644
--- a/lib/gitlab/metrics/system.rb
+++ b/lib/gitlab/metrics/system.rb
@@ -30,6 +30,17 @@ module Gitlab
           0
         end
       end
+
+      # THREAD_CPUTIME is not supported on OS X
+      if Process.const_defined?(:CLOCK_THREAD_CPUTIME_ID)
+        def self.cpu_time
+          Process.clock_gettime(Process::CLOCK_THREAD_CPUTIME_ID, :millisecond)
+        end
+      else
+        def self.cpu_time
+          Process.clock_gettime(Process::CLOCK_PROCESS_CPUTIME_ID, :millisecond)
+        end
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb
index 8f63a5f2043..edefec909c9 100644
--- a/spec/lib/gitlab/metrics_spec.rb
+++ b/spec/lib/gitlab/metrics_spec.rb
@@ -79,19 +79,13 @@ describe Gitlab::Metrics do
       end
 
       it 'adds a metric to the current transaction' do
-        expect(transaction).to receive(:add_metric).
-          with(:foo, { duration: a_kind_of(Numeric) }, { tag: 'value' })
+        expect(transaction).to receive(:increment).
+          with('foo_real_time', a_kind_of(Numeric))
 
-        Gitlab::Metrics.measure(:foo, {}, tag: 'value') { 10 }
-      end
-
-      it 'supports adding of custom values' do
-        values = { duration: a_kind_of(Numeric), number: 10 }
-
-        expect(transaction).to receive(:add_metric).
-          with(:foo, values, { tag: 'value' })
+        expect(transaction).to receive(:increment).
+          with('foo_cpu_time', a_kind_of(Numeric))
 
-        Gitlab::Metrics.measure(:foo, { number: 10 }, tag: 'value') { 10 }
+        Gitlab::Metrics.measure(:foo) { 10 }
       end
 
       it 'returns the return value of the block' do
-- 
cgit v1.2.1


From 185d78bcb3064c25b764937949d2bb66ec66901a Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Mon, 11 Apr 2016 13:11:13 +0200
Subject: Added specs for Gitlab::Metrics::System.cpu_time

---
 spec/lib/gitlab/metrics/system_spec.rb | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/spec/lib/gitlab/metrics/system_spec.rb b/spec/lib/gitlab/metrics/system_spec.rb
index f8c1d956ca1..d6ae54e25e8 100644
--- a/spec/lib/gitlab/metrics/system_spec.rb
+++ b/spec/lib/gitlab/metrics/system_spec.rb
@@ -26,4 +26,10 @@ describe Gitlab::Metrics::System do
       end
     end
   end
+
+  describe '.cpu_time' do
+    it 'returns a Fixnum' do
+      expect(described_class.cpu_time).to be_an_instance_of(Fixnum)
+    end
+  end
 end
-- 
cgit v1.2.1


From d9110a7ecab52ab0716a42c2075cebdf8028d5e7 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Mon, 11 Apr 2016 13:27:24 +0200
Subject: Track call counts in Gitlab::Metrics.measure_block

---
 doc/development/instrumentation.md | 7 ++++---
 lib/gitlab/metrics.rb              | 1 +
 spec/lib/gitlab/metrics_spec.rb    | 3 +++
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/doc/development/instrumentation.md b/doc/development/instrumentation.md
index f7e148fb3e4..c1cf2e77c26 100644
--- a/doc/development/instrumentation.md
+++ b/doc/development/instrumentation.md
@@ -14,10 +14,11 @@ Gitlab::Metrics.measure(:foo) do
 end
 ```
 
-Two values are measured for a block:
+3 values are measured for a block:
 
-1. The real time elapsed, stored in NAME_real_time
-2. The CPU time elapsed, stored in NAME_cpu_time
+1. The real time elapsed, stored in NAME_real_time.
+2. The CPU time elapsed, stored in NAME_cpu_time.
+3. The call count, stored in NAME_call_count.
 
 Both the real and CPU timings are measured in milliseconds.
 
diff --git a/lib/gitlab/metrics.rb b/lib/gitlab/metrics.rb
index 4d64555027e..33dd3e39f4d 100644
--- a/lib/gitlab/metrics.rb
+++ b/lib/gitlab/metrics.rb
@@ -97,6 +97,7 @@ module Gitlab
 
       Transaction.current.increment("#{name}_real_time", real_time)
       Transaction.current.increment("#{name}_cpu_time", cpu_time)
+      Transaction.current.increment("#{name}_call_count", 1)
 
       retval
     end
diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb
index edefec909c9..a3b68455260 100644
--- a/spec/lib/gitlab/metrics_spec.rb
+++ b/spec/lib/gitlab/metrics_spec.rb
@@ -85,6 +85,9 @@ describe Gitlab::Metrics do
         expect(transaction).to receive(:increment).
           with('foo_cpu_time', a_kind_of(Numeric))
 
+        expect(transaction).to receive(:increment).
+          with('foo_call_count', 1)
+
         Gitlab::Metrics.measure(:foo) { 10 }
       end
 
-- 
cgit v1.2.1


From 7eed4608fe5adf65d6a29ef50c93485ca2e6806f Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Mon, 11 Apr 2016 14:29:38 +0200
Subject: Fixed stubbing for Gitlab::Metrics specs

If the measure method uses Transaction.current directly the SQL
subscriber (Subscribers::ActiveRecord) will add timings of queries
triggered by DB cleaner.
---
 lib/gitlab/metrics.rb           | 16 ++++++++++++----
 spec/lib/gitlab/metrics_spec.rb |  2 +-
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/lib/gitlab/metrics.rb b/lib/gitlab/metrics.rb
index 33dd3e39f4d..2a0a5629be5 100644
--- a/lib/gitlab/metrics.rb
+++ b/lib/gitlab/metrics.rb
@@ -82,7 +82,9 @@ module Gitlab
     #
     # Returns the value yielded by the supplied block.
     def self.measure(name)
-      return yield unless Transaction.current
+      trans = current_transaction
+
+      return yield unless trans
 
       real_start = Time.now.to_f
       cpu_start = System.cpu_time
@@ -95,9 +97,9 @@ module Gitlab
       real_time = (real_stop - real_start) * 1000.0
       cpu_time = cpu_stop - cpu_start
 
-      Transaction.current.increment("#{name}_real_time", real_time)
-      Transaction.current.increment("#{name}_cpu_time", cpu_time)
-      Transaction.current.increment("#{name}_call_count", 1)
+      trans.increment("#{name}_real_time", real_time)
+      trans.increment("#{name}_cpu_time", cpu_time)
+      trans.increment("#{name}_call_count", 1)
 
       retval
     end
@@ -113,5 +115,11 @@ module Gitlab
           new(udp: { host: host, port: port })
       end
     end
+
+    private
+
+    def self.current_transaction
+      Transaction.current
+    end
   end
 end
diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb
index a3b68455260..3dee13e27f4 100644
--- a/spec/lib/gitlab/metrics_spec.rb
+++ b/spec/lib/gitlab/metrics_spec.rb
@@ -74,7 +74,7 @@ describe Gitlab::Metrics do
       let(:transaction) { Gitlab::Metrics::Transaction.new }
 
       before do
-        allow(Gitlab::Metrics::Transaction).to receive(:current).
+        allow(Gitlab::Metrics).to receive(:current_transaction).
           and_return(transaction)
       end
 
-- 
cgit v1.2.1


From ca0b3ea1f107562492babbaf5ce5fec43897bd88 Mon Sep 17 00:00:00 2001
From: Jacob Vosmaer <contact@jacobvosmaer.nl>
Date: Mon, 11 Apr 2016 17:03:27 +0200
Subject: Go back to gitlab-workhorse 0.7.1

We found out that 0.7.2 breaks shallow `git clone`. GitLab 8.7 will
work fine with gitlab-workhorse 0.7.1 too.
---
 GITLAB_WORKHORSE_VERSION    | 2 +-
 doc/install/installation.md | 2 +-
 doc/update/8.6-to-8.7.md    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION
index 7486fdbc50b..39e898a4f95 100644
--- a/GITLAB_WORKHORSE_VERSION
+++ b/GITLAB_WORKHORSE_VERSION
@@ -1 +1 @@
-0.7.2
+0.7.1
diff --git a/doc/install/installation.md b/doc/install/installation.md
index d97dc7d1311..f8f7d6a9ebe 100644
--- a/doc/install/installation.md
+++ b/doc/install/installation.md
@@ -352,7 +352,7 @@ GitLab Shell is an SSH access and repository management software developed speci
     cd /home/git
     sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-workhorse.git
     cd gitlab-workhorse
-    sudo -u git -H git checkout v0.7.2
+    sudo -u git -H git checkout v0.7.1
     sudo -u git -H make
 
 ### Initialize Database and Activate Advanced Features
diff --git a/doc/update/8.6-to-8.7.md b/doc/update/8.6-to-8.7.md
index 57847d2d9fd..8599133a726 100644
--- a/doc/update/8.6-to-8.7.md
+++ b/doc/update/8.6-to-8.7.md
@@ -58,7 +58,7 @@ GitLab 8.1.
 ```bash
 cd /home/git/gitlab-workhorse
 sudo -u git -H git fetch --all
-sudo -u git -H git checkout v0.7.2
+sudo -u git -H git checkout v0.7.1
 sudo -u git -H make
 ```
 
-- 
cgit v1.2.1


From 935f913165b91467a70d9ba2b0ea29fad467db9d Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Mon, 11 Apr 2016 17:42:12 +0200
Subject: Instrument Banzai code

---
 config/initializers/metrics.rb | 23 +++++++++++++++++++++++
 lib/banzai/renderer.rb         | 20 ++++++++++++--------
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb
index a9fc38fb04a..1b445bbbd10 100644
--- a/config/initializers/metrics.rb
+++ b/config/initializers/metrics.rb
@@ -75,6 +75,29 @@ if Gitlab::Metrics.enabled?
       config.instrument_methods(const)
       config.instrument_instance_methods(const)
     end
+
+    # Instruments all Banzai filters
+    Dir[Rails.root.join('lib', 'banzai', 'filter', '*.rb')].each do |file|
+      klass = File.basename(file, File.extname(file)).camelize
+      const = Banzai::Filter.const_get(klass)
+
+      config.instrument_methods(const)
+      config.instrument_instance_methods(const)
+    end
+
+    config.instrument_methods(Banzai::ReferenceExtractor)
+    config.instrument_instance_methods(Banzai::ReferenceExtractor)
+
+    config.instrument_methods(Banzai::Renderer)
+    config.instrument_methods(Banzai::Querying)
+
+    [Issuable, Mentionable, Participable].each do |klass|
+      config.instrument_instance_methods(klass)
+      config.instrument_instance_methods(klass::ClassMethods)
+    end
+
+    config.instrument_methods(Gitlab::ReferenceExtractor)
+    config.instrument_instance_methods(Gitlab::ReferenceExtractor)
   end
 
   GC::Profiler.enable
diff --git a/lib/banzai/renderer.rb b/lib/banzai/renderer.rb
index ae714c87dc5..c14a9c4c722 100644
--- a/lib/banzai/renderer.rb
+++ b/lib/banzai/renderer.rb
@@ -19,8 +19,10 @@ module Banzai
       cache_key = full_cache_key(cache_key, context[:pipeline])
 
       if cache_key
-        Rails.cache.fetch(cache_key) do
-          cacheless_render(text, context)
+        Gitlab::Metrics.measure(:banzai_cached_render) do
+          Rails.cache.fetch(cache_key) do
+            cacheless_render(text, context)
+          end
         end
       else
         cacheless_render(text, context)
@@ -64,13 +66,15 @@ module Banzai
     private
 
     def self.cacheless_render(text, context = {})
-      result = render_result(text, context)
+      Gitlab::Metrics.measure(:banzai_cacheless_render) do
+        result = render_result(text, context)
 
-      output = result[:output]
-      if output.respond_to?(:to_html)
-        output.to_html
-      else
-        output.to_s
+        output = result[:output]
+        if output.respond_to?(:to_html)
+          output.to_html
+        else
+          output.to_s
+        end
       end
     end
 
-- 
cgit v1.2.1