Merge branch 'release/7.9.1' into '7-9-stable'

WIP | Release 7.9.1

See merge request !1738

20 files changed, 180 insertions, 25 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 543c382a8e7..b782ad4c42d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,15 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
-v 7.9.0 (unreleased)
+v 7.9.1
+  - Include missing events and fix save functionality in admin service template settings form (Stan Hu)
+  - Fix "Import projects from" button to show the correct instructions (Stan Hu)
+  - Fix OAuth2 issue importing a new project from GitHub and GitLab (Stan Hu)
+  - Fix for LDAP with commas in DN
+  - Fix missing events and in admin Slack service template settings form (Stan Hu)
+  - Don't show commit comment button when user is not signed in.
+  - Downgrade gemnasium-gitlab-service gem
+
+v 7.9.0
   - Add HipChat integration documentation (Stan Hu)
   - Update documentation for object_kind field in Webhook push and tag push Webhooks (Stan Hu)
   - Fix broken email images (Hannes Rosenögger)
diff --git a/Gemfile.lock b/Gemfile.lock
index 3fd051e29b5..c38daf648de 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -188,7 +188,7 @@ GEM
       dotenv (>= 0.7)
       thor (>= 0.13.6)
     formatador (0.2.4)
-    gemnasium-gitlab-service (0.2.5)
+    gemnasium-gitlab-service (0.2.4)
       rugged (~> 0.21)
     gemojione (2.0.0)
       json
diff --git a/app/controllers/admin/services_controller.rb b/app/controllers/admin/services_controller.rb
index 44a3f1379d8..76a938c5fe4 100644
--- a/app/controllers/admin/services_controller.rb
+++ b/app/controllers/admin/services_controller.rb
@@ -46,7 +46,9 @@ class Admin::ServicesController < Admin::ApplicationController
         :user_key, :device, :priority, :sound, :bamboo_url, :username, :password,
         :build_key, :server, :teamcity_url, :build_type,
         :description, :issues_url, :new_issue_url, :restrict_to_branch,
-        :send_from_committer_email, :disable_diffs
+        :send_from_committer_email, :disable_diffs,
+        :push_events, :tag_push_events, :note_events, :issues_events,
+        :merge_requests_events
     ])
   end
 end
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 2225b110651..a256f3965a3 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -146,6 +146,10 @@ module ProjectsHelper
       nav_tabs << feature if project.send :"#{feature}_enabled"
     end
 
+    if project.issues_enabled || project.merge_requests_enabled
+      nav_tabs << [:milestones, :labels]
+    end
+
     nav_tabs.flatten
   end
 
diff --git a/app/views/admin/services/_form.html.haml b/app/views/admin/services/_form.html.haml
index a953833b37c..18b7e8ba270 100644
--- a/app/views/admin/services/_form.html.haml
+++ b/app/views/admin/services/_form.html.haml
@@ -14,6 +14,11 @@
       = preserve do
         = markdown @service.help
 
+  .form-group
+    = f.label :active, "Active", class: "control-label"
+    .col-sm-10
+      = f.check_box :active
+
   - if @service.supported_events.length > 1
     .form-group
       = f.label :url, "Trigger", class: 'control-label'
@@ -34,6 +39,14 @@
                 %strong Tag push events
               %p.light
                 This url will be triggered when a new tag is pushed to the repository
+        - if @service.supported_events.include?("note")
+          %div
+            = f.check_box :note_events, class: 'pull-left'
+            .prepend-left-20
+              = f.label :note_events, class: 'list-label' do
+                %strong Comments
+              %p.light
+                This url will be triggered when someone adds a comment
         - if @service.supported_events.include?("issue")
           %div
             = f.check_box :issues_events, class: 'pull-left'
diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml
index d340ab1796a..c58161c3362 100644
--- a/app/views/layouts/nav/_project.html.haml
+++ b/app/views/layouts/nav/_project.html.haml
@@ -44,11 +44,12 @@
           %span
             Graphs
 
-    = nav_link(controller: :milestones) do
-      = link_to namespace_project_milestones_path(@project.namespace, @project), title: 'Milestones' do
-        %i.fa.fa-clock-o
-        %span
-          Milestones
+    - if project_nav_tab? :milestones
+      = nav_link(controller: :milestones) do
+        = link_to namespace_project_milestones_path(@project.namespace, @project), title: 'Milestones' do
+          %i.fa.fa-clock-o
+          %span
+            Milestones
 
     - if project_nav_tab? :issues
       = nav_link(controller: :issues) do
@@ -67,11 +68,12 @@
             Merge Requests
             %span.count.merge_counter= @project.merge_requests.opened.count
 
-    = nav_link(controller: :labels) do
-      = link_to namespace_project_labels_path(@project.namespace, @project), title: 'Labels' do
-        %i.fa.fa-tags
-        %span
-          Labels
+    - if project_nav_tab? :labels
+      = nav_link(controller: :labels) do
+        = link_to namespace_project_labels_path(@project.namespace, @project), title: 'Labels' do
+          %i.fa.fa-tags
+          %span
+            Labels
 
     - if project_nav_tab? :wiki
       = nav_link(controller: :wikis) do
diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml
index b1c987563f1..e691db9c08e 100644
--- a/app/views/projects/diffs/_text_file.html.haml
+++ b/app/views/projects/diffs/_text_file.html.haml
@@ -16,7 +16,7 @@
       - else
         %td.old_line
           = link_to raw(type == "new" ? "&nbsp;" : line_old), "##{line_code}", id: line_code
-          - if @comments_allowed
+          - if @comments_allowed && can?(current_user, :write_note, @project)
             = link_to_new_diff_note(line_code)
         %td.new_line{data: {linenumber: line.new_pos}}
           = link_to raw(type == "old" ? "&nbsp;" : line.new_pos) , "##{line_code}", id: line_code
diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml
index 110d8967342..25cc0030965 100644
--- a/app/views/projects/milestones/show.html.haml
+++ b/app/views/projects/milestones/show.html.haml
@@ -60,11 +60,12 @@
       Participants
       %span.badge= @users.count
 
-  .pull-right
-    = link_to new_namespace_project_issue_path(@project.namespace, @project, issue: { milestone_id: @milestone.id }), class: "btn  btn-grouped", title: "New Issue" do
-      %i.fa.fa-plus
-      New Issue
-    = link_to 'Browse Issues', namespace_project_issues_path(@milestone.project.namespace, @milestone.project, milestone_id: @milestone.id), class: "btn  edit-milestone-link btn-grouped"
+  - if @project.issues_enabled
+    .pull-right
+      = link_to new_namespace_project_issue_path(@project.namespace, @project, issue: { milestone_id: @milestone.id }), class: "btn  btn-grouped", title: "New Issue" do
+        %i.fa.fa-plus
+        New Issue
+      = link_to 'Browse Issues', namespace_project_issues_path(@milestone.project.namespace, @milestone.project, milestone_id: @milestone.id), class: "btn  edit-milestone-link btn-grouped"
 
 .tab-content
   .tab-pane.active#tab-issues
diff --git a/app/views/projects/new.html.haml b/app/views/projects/new.html.haml
index 7fc612c0c7d..dbceec3633c 100644
--- a/app/views/projects/new.html.haml
+++ b/app/views/projects/new.html.haml
@@ -111,6 +111,6 @@
   $ ->
     $('.how_to_import_link').bind 'click', (e) ->
       e.preventDefault()
-      import_modal = $(this).parent().find(".modal").show()
+      import_modal = $(this).next(".modal").show()
     $('.modal-header .close').bind 'click', ->
       $(".modal").hide()
diff --git a/features/admin/settings.feature b/features/admin/settings.feature
index 8fdf0575c2c..52e47307b23 100644
--- a/features/admin/settings.feature
+++ b/features/admin/settings.feature
@@ -7,3 +7,10 @@ Feature: Admin Settings
   Scenario: Change application settings
     When I modify settings and save form
     Then I should see application settings saved
+
+  Scenario: Change Slack Service Template settings
+    When I click on "Service Templates"
+    And I click on "Slack" service
+    Then I check all events and submit form
+    And I should see service template settings saved
+    And I should see all checkboxes checked
diff --git a/features/dashboard/new_project.feature b/features/dashboard/new_project.feature
new file mode 100644
index 00000000000..431dc4ccfcb
--- /dev/null
+++ b/features/dashboard/new_project.feature
@@ -0,0 +1,13 @@
+@dashboard
+Feature: New Project
+Background:
+  Given I sign in as a user
+  And I own project "Shop"
+  And I visit dashboard page
+
+  @javascript
+  Scenario: I should see New projects page
+  Given I click "New project" link
+  Then I see "New project" page
+  When I click on "Import project from GitHub"
+  Then I see instructions on how to import from GitHub
diff --git a/features/steps/admin/settings.rb b/features/steps/admin/settings.rb
index c2d0d2a3fa3..87d4e969ff5 100644
--- a/features/steps/admin/settings.rb
+++ b/features/steps/admin/settings.rb
@@ -15,4 +15,33 @@ class Spinach::Features::AdminSettings < Spinach::FeatureSteps
     current_application_settings.home_page_url.should == 'https://about.gitlab.com/'
     page.should have_content 'Application settings saved successfully'
   end
+
+  step 'I click on "Service Templates"' do
+    click_link 'Service Templates'
+  end
+
+  step 'I click on "Slack" service' do
+    click_link 'Slack'
+  end
+
+  step 'I check all events and submit form' do
+    page.check('Active')
+    page.check('Push events')
+    page.check('Tag push events')
+    page.check('Comments')
+    page.check('Issues events')
+    page.check('Merge Request events')
+    fill_in 'Webhook', with: "http://localhost"
+    click_on 'Save'
+  end
+
+  step 'I should see service template settings saved' do
+    page.should have_content 'Application settings saved successfully'
+  end
+
+  step 'I should see all checkboxes checked' do
+    all('input[type=checkbox]').each do |checkbox|
+      checkbox.should be_checked
+    end
+  end
 end
diff --git a/features/steps/dashboard/new_project.rb b/features/steps/dashboard/new_project.rb
new file mode 100644
index 00000000000..5e588ceb780
--- /dev/null
+++ b/features/steps/dashboard/new_project.rb
@@ -0,0 +1,27 @@
+class Spinach::Features::NewProject < Spinach::FeatureSteps
+  include SharedAuthentication
+  include SharedPaths
+  include SharedProject
+
+  step 'I click "New project" link' do
+    click_link "New project"
+  end
+
+  step 'I see "New project" page' do
+    page.should have_content("Project path")
+  end
+
+  step 'I click on "Import project from GitHub"' do
+    first('.how_to_import_link').click
+  end
+
+  step 'I see instructions on how to import from GitHub' do
+    github_modal = first('.modal-body')
+    github_modal.should be_visible
+    github_modal.should have_content "To enable importing projects from GitHub"
+
+    all('.modal-body').each do |element|
+      element.should_not be_visible unless element == github_modal
+    end
+  end
+end
diff --git a/lib/gitlab/bitbucket_import/client.rb b/lib/gitlab/bitbucket_import/client.rb
index 1e4906c9e31..5b1952b9675 100644
--- a/lib/gitlab/bitbucket_import/client.rb
+++ b/lib/gitlab/bitbucket_import/client.rb
@@ -62,7 +62,7 @@ module Gitlab
       end
 
       def find_deploy_key(project_identifier, key)
-        JSON.parse(api.get("/api/1.0/repositories/#{project_identifier}/deploy-keys").body).find do |deploy_key| 
+        JSON.parse(api.get("/api/1.0/repositories/#{project_identifier}/deploy-keys").body).find do |deploy_key|
           deploy_key["key"].chomp == key.chomp
         end
       end
@@ -92,7 +92,7 @@ module Gitlab
       end
 
       def bitbucket_options
-        OmniAuth::Strategies::Bitbucket.default_options[:client_options].dup
+        OmniAuth::Strategies::Bitbucket.default_options[:client_options].symbolize_keys
       end
     end
   end
diff --git a/lib/gitlab/github_import/client.rb b/lib/gitlab/github_import/client.rb
index 7fe076b333b..270cbcd9ccd 100644
--- a/lib/gitlab/github_import/client.rb
+++ b/lib/gitlab/github_import/client.rb
@@ -46,7 +46,7 @@ module Gitlab
       end
 
       def github_options
-        OmniAuth::Strategies::GitHub.default_options[:client_options].dup
+        OmniAuth::Strategies::GitHub.default_options[:client_options].symbolize_keys
       end
     end
   end
diff --git a/lib/gitlab/gitlab_import/client.rb b/lib/gitlab/gitlab_import/client.rb
index 2236439c6ce..f48ede9d067 100644
--- a/lib/gitlab/gitlab_import/client.rb
+++ b/lib/gitlab/gitlab_import/client.rb
@@ -71,7 +71,7 @@ module Gitlab
       end
 
       def gitlab_options
-        OmniAuth::Strategies::GitLab.default_options[:client_options].dup
+        OmniAuth::Strategies::GitLab.default_options[:client_options].symbolize_keys
       end
     end
   end
diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb
index 3c426179375..b81f3e8e8f5 100644
--- a/lib/gitlab/ldap/person.rb
+++ b/lib/gitlab/ldap/person.rb
@@ -14,7 +14,6 @@ module Gitlab
       end
 
       def self.find_by_dn(dn, adapter)
-        dn = Net::LDAP::Filter.escape(dn)
         adapter.user('dn', dn)
       end
 
diff --git a/spec/lib/gitlab/bitbucket_import/client_spec.rb b/spec/lib/gitlab/bitbucket_import/client_spec.rb
new file mode 100644
index 00000000000..dd450e9967b
--- /dev/null
+++ b/spec/lib/gitlab/bitbucket_import/client_spec.rb
@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe Gitlab::BitbucketImport::Client do
+  let(:token) { '123456' }
+  let(:secret) { 'secret' }
+  let(:client) { Gitlab::BitbucketImport::Client.new(token, secret) }
+
+  before do
+    Gitlab.config.omniauth.providers << OpenStruct.new(app_id: "asd123", app_secret: "asd123", name: "bitbucket")
+  end
+
+  it 'all OAuth client options are symbols' do
+    client.consumer.options.keys.each do |key|
+      expect(key).to be_kind_of(Symbol)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb
new file mode 100644
index 00000000000..26618120316
--- /dev/null
+++ b/spec/lib/gitlab/github_import/client_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe Gitlab::GithubImport::Client do
+  let(:token) { '123456' }
+  let(:client) { Gitlab::GithubImport::Client.new(token) }
+
+  before do
+    Gitlab.config.omniauth.providers << OpenStruct.new(app_id: "asd123", app_secret: "asd123", name: "github")
+  end
+
+  it 'all OAuth2 client options are symbols' do
+    client.client.options.keys.each do |key|
+      expect(key).to be_kind_of(Symbol)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/gitlab_import/client_spec.rb b/spec/lib/gitlab/gitlab_import/client_spec.rb
new file mode 100644
index 00000000000..c511c515474
--- /dev/null
+++ b/spec/lib/gitlab/gitlab_import/client_spec.rb
@@ -0,0 +1,16 @@
+require 'spec_helper'
+
+describe Gitlab::GitlabImport::Client do
+  let(:token) { '123456' }
+  let(:client) { Gitlab::GitlabImport::Client.new(token) }
+
+  before do
+    Gitlab.config.omniauth.providers << OpenStruct.new(app_id: "asd123", app_secret: "asd123", name: "gitlab")
+  end
+
+  it 'all OAuth2 client options are symbols' do
+    client.client.options.keys.each do |key|
+      expect(key).to be_kind_of(Symbol)
+    end
+  end
+end