Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce

CHANGELOG

 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.9.0 (unreleased)
+  - Fix builds API response not including commit data
   - Fix error when CI job variables key specified but not defined
   - Fix pipeline status when there are no builds in pipeline
   - Fix Error 500 when using closes_issues API with an external issue tracker
@@ -8,6 +9,7 @@ v 8.9.0 (unreleased)
   - Bulk assign/unassign labels to issues.
   - Ability to prioritize labels !4009 / !3205 (Thijs Wouters)
   - Show Star and Fork buttons on mobile.
+  - Performance improvements on RelativeLinkFilter
   - Fix endless redirections when accessing user OAuth applications when they are disabled
   - Allow enabling wiki page events from Webhook management UI
   - Bump rouge to 1.11.0
@@ -145,6 +147,7 @@ v 8.9.0 (unreleased)
   - Cache user todo counts from TodoService
   - Ensure Todos counters doesn't count Todos for projects pending delete
   - Add left/right arrows horizontal navigation
+  - Add tooltip to pin/unpin navbar
 
 v 8.8.5
   - Import GitHub repositories respecting the API rate limit !4166

app/assets/javascripts/application.js.coffee

@@ -268,17 +268,33 @@ $ ->
     .on 'click', '.js-nav-pin', (e) ->
       e.preventDefault()
 
+      $pinBtn = $(e.currentTarget)
+      $page = $ '.page-with-sidebar'
+      $topNav = $ '.navbar-fixed-top'
+      $tooltip = $ "##{$pinBtn.attr('aria-describedby')}"
+      doPinNav = not $page.is('.page-sidebar-pinned')
+      tooltipText = 'Pin navigation'
+
       $(this).toggleClass 'is-active'
 
-      if $.cookie('pin_nav') is 'true'
-        $.cookie 'pin_nav', 'false', { path: '/' }
-        $('.page-with-sidebar')
-          .removeClass('page-sidebar-pinned')
-          .toggleClass('page-sidebar-collapsed page-sidebar-expanded')
-        $('.navbar-fixed-top')
-          .removeClass('header-pinned-nav')
-          .toggleClass('header-collapsed header-expanded')
+      if doPinNav
+        $page.addClass('page-sidebar-pinned')
+        $topNav.addClass('header-pinned-nav')
       else
-        $.cookie 'pin_nav', 'true', { path: '/' }
-        $('.page-with-sidebar').addClass('page-sidebar-pinned')
-        $('.navbar-fixed-top').addClass('header-pinned-nav')
+        $tooltip.remove() # Remove it immediately when collapsing the sidebar
+        $page.removeClass('page-sidebar-pinned')
+             .toggleClass('page-sidebar-collapsed page-sidebar-expanded')
+        $topNav.removeClass('header-pinned-nav')
+               .toggleClass('header-collapsed header-expanded')
+
+      # Save settings
+      $.cookie 'pin_nav', doPinNav, { path: '/' }
+
+      if $.cookie('pin_nav') is 'true' or doPinNav
+        tooltipText = 'Unpin navigation'
+
+      # Update tooltip text immediately
+      $tooltip.find('.tooltip-inner').text(tooltipText)
+
+      # Persist tooltip title
+      $pinBtn.attr('title', tooltipText).tooltip('fixTitle')

app/assets/javascripts/notifications_dropdown.js.coffee

 class @NotificationsDropdown
-  $ ->
+  constructor: ->
     $(document)
       .off 'click', '.update-notification'
       .on 'click', '.update-notification', (e) ->
@@ -18,7 +18,8 @@ class @NotificationsDropdown
       .off 'ajax:success', '.notification-form'
       .on 'ajax:success', '.notification-form', (e, data) ->
         if data.saved
-          new Flash('Notification settings saved', 'notice')
-          $(e.currentTarget).closest('.notification-dropdown').replaceWith(data.html)
+          $(e.currentTarget)
+            .closest('.notification-dropdown')
+            .replaceWith(data.html)
         else
           new Flash('Failed to save new settings', 'alert')

app/assets/javascripts/project.js.coffee

@@ -35,7 +35,6 @@ class @Project
       $(@).parents('.no-password-message').remove()
       e.preventDefault()
 
-
     @projectSelectDropdown()
 
   projectSelectDropdown: ->

app/assets/stylesheets/framework.scss

@@ -37,3 +37,4 @@
 @import "framework/timeline.scss";
 @import "framework/typography.scss";
 @import "framework/zen.scss";
+@import "framework/blank";

app/assets/stylesheets/framework/blank.scss

+.blank-state {
+  padding-top: 20px;
+  padding-bottom: 20px;
+  text-align: center;
+}
+
+.blank-state-no-icon {
+  padding-top: 40px;
+  padding-bottom: 40px;  
+}
+
+.blank-state-title {
+  margin-top: 0;
+  margin-bottom: 5px;
+  font-size: 19px;
+  font-weight: normal;
+}
+
+.blank-state-text {
+  margin-top: 0;
+  margin-bottom: $gl-padding;
+  font-size: 15px;
+}

app/assets/stylesheets/pages/projects.scss

@@ -101,7 +101,8 @@
 
   .notifications-btn {
 
-    .fa-bell {
+    .fa-bell,
+    .fa-spinner {
       margin-right: 6px;
     }
 

app/controllers/projects/pipelines_controller.rb

@@ -54,6 +54,6 @@ class Projects::PipelinesController < Projects::ApplicationController
   end
 
   def commit
-    @commit ||= @pipeline.commit_data
+    @commit ||= @pipeline.commit
   end
 end

app/models/ci/pipeline.rb

@@ -37,22 +37,22 @@ module Ci
     end
 
     def git_author_name
-      commit_data.author_name if commit_data
+      commit.try(:author_name)
     end
 
     def git_author_email
-      commit_data.author_email if commit_data
+      commit.try(:author_email)
     end
 
     def git_commit_message
-      commit_data.message if commit_data
+      commit.try(:message)
     end
 
     def short_sha
       Ci::Pipeline.truncate_sha(sha)
     end
 
-    def commit_data
+    def commit
       @commit ||= project.commit(sha)
     rescue
       nil

app/models/commit.rb

@@ -271,6 +271,32 @@ class Commit
     merged_merge_request ? 'merge request' : 'commit'
   end
 
+  # Get the URI type of the given path
+  #
+  # Used to build URLs to files in the repository in GFM.
+  #
+  # path - String path to check
+  #
+  # Examples:
+  #
+  #   uri_type('doc/README.md') # => :blob
+  #   uri_type('doc/logo.png')  # => :raw
+  #   uri_type('doc/api')       # => :tree
+  #   uri_type('not/found')     # => :nil
+  #
+  # Returns a symbol
+  def uri_type(path)
+    entry = @raw.tree.path(path)
+    if entry[:type] == :blob
+      blob = Gitlab::Git::Blob.new(name: entry[:name])
+      blob.image? ? :raw : :blob
+    else
+      entry[:type]
+    end
+  rescue Rugged::TreeError
+    nil
+  end
+
   private
 
   def repo_changes

app/models/commit_status.rb

@@ -8,6 +8,8 @@ class CommitStatus < ActiveRecord::Base
   belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id, touch: true
   belongs_to :user
 
+  delegate :commit, to: :pipeline
+
   validates :pipeline, presence: true, unless: :importing?
 
   validates_presence_of :name

app/views/layouts/_page.html.haml

@@ -13,7 +13,7 @@
         = image_tag avatar_icon(current_user, 60), alt: 'Profile', class: 'avatar avatar s36'
         .username
           = current_user.username
-    = link_to '#', class: "nav-header-btn text-center pin-nav-btn #{'is-active' if pinned_nav?} js-nav-pin", title: 'Pin/Unpin navigation' do
+    = link_to '#', class: "nav-header-btn text-center pin-nav-btn has-tooltip #{'is-active' if pinned_nav?} js-nav-pin", title: pinned_nav? ? "Unpin navigation" : "Pin Navigation", data: {placement: 'right', container: 'body'} do
       %span.sr-only Toggle navigation pinning
       = icon('thumb-tack')
   - if defined?(nav) && nav

app/views/profiles/notifications/show.html.haml

@@ -28,7 +28,7 @@
       = label_tag :global_notification_level, "Global notification level", class: "label-light"
       %br
       .clearfix
-      .form-group.pull-left
+      .form-group.pull-left.global-notification-setting
         = render 'shared/notifications/button', notification_setting: @global_notification_setting, left_align: true
 
       .clearfix

app/views/projects/ci/pipelines/_pipeline.html.haml

@@ -24,8 +24,8 @@
         %span.label.label-warning stuck
 
       %p.commit-title
-        - if commit_data = pipeline.commit_data
-          = link_to_gfm truncate(commit_data.title, length: 60), namespace_project_commit_path(@project.namespace, @project, commit_data.id), class: "commit-row-message"
+        - if commit = pipeline.commit
+          = link_to_gfm truncate(commit.title, length: 60), namespace_project_commit_path(@project.namespace, @project, commit.id), class: "commit-row-message"
         - else
           Cant find HEAD commit for this branch
 

app/views/projects/environments/index.html.haml

@@ -3,16 +3,24 @@
 = render "projects/pipelines/head"
 
 %div{ class: (container_class) }
-  - if can?(current_user, :create_environment, @project)
+  - if can?(current_user, :create_environment, @project) && !@environments.blank?
     .top-area
       .nav-controls
         = link_to new_namespace_project_environment_path(@project.namespace, @project), class: 'btn btn-create' do
           New environment
 
   - if @environments.blank?
-    %ul.content-list.environments
-      %li.nothing-here-block
-        No environments to show
+    .blank-state.blank-state-no-icon
+      %h2.blank-state-title
+        You don't have any environments right now.
+      %p.blank-state-text
+        Environments are places where code gets deployed, such as staging or production.
+        %br
+        = succeed "." do
+          = link_to "Read more about environments", help_page_path("ci", "environments")
+      - if can?(current_user, :create_environment, @project)
+        = link_to new_namespace_project_environment_path(@project.namespace, @project), class: 'btn btn-create' do
+          New environment
   - else
     .table-holder
       %table.table.environments

app/views/projects/environments/new.html.haml

@@ -4,6 +4,9 @@
   .col-lg-3
     %h4.prepend-top-0
       New Environment
-    %p Environments allow you to track deployments of your application
+    %p
+      Environments allow you to track deployments of your application
+      = succeed "." do
+        = link_to "Read more about environments", help_page_path("ci", "environments")
 
   = render 'form'

app/views/projects/environments/show.html.haml

@@ -13,10 +13,14 @@
           = link_to 'Destroy', namespace_project_environment_path(@project.namespace, @project, @environment), data: { confirm: 'Are you sure you want to delete this environment?' }, class: 'btn btn-danger', method: :delete
 
   - if @deployments.blank?
-    %ul.content-list.environments
-      %li.nothing-here-block
-        No deployments for
-        %strong= @environment.name
+    .blank-state.blank-state-no-icon
+      %h2.blank-state-title
+        You don't have any deployments right now.
+      %p.blank-state-text
+        Define environments in the deploy stage(s) in
+        %code .gitlab-ci.yml
+        to track deployments here.
+      = link_to "Read more", help_page_path("ci", "environments"), class: "btn btn-success"
   - else
     .table-holder
       %table.table.environments

features/steps/profile/notifications.rb

@@ -15,8 +15,6 @@ class Spinach::Features::ProfileNotifications < Spinach::FeatureSteps
   end
 
   step 'I should see Notification saved message' do
-    page.within '.flash-container' do
-      expect(page).to have_content 'Notification settings saved'
-    end
+    expect(page).to have_content 'On mention'
   end
 end

features/steps/project/project.rb

@@ -134,8 +134,8 @@ class Spinach::Features::Project < Spinach::FeatureSteps
   end
 
   step 'I should see Notification saved message' do
-    page.within '.flash-container' do
-      expect(page).to have_content 'Notification settings saved'
+    page.within '#notifications-button' do
+      expect(page).to have_content 'On mention'
     end
   end
 

lib/api/entities.rb

@@ -445,11 +445,7 @@ module API
       expose :created_at, :started_at, :finished_at
       expose :user, with: User
       expose :artifacts_file, using: BuildArtifactFile, if: -> (build, opts) { build.artifacts? }
-      expose :commit, with: RepoCommit do |repo_obj, _options|
-        if repo_obj.respond_to?(:commit)
-          repo_obj.commit.commit_data
-        end
-      end
+      expose :commit, with: RepoCommit
       expose :runner, with: Runner
     end
 

lib/banzai/filter/relative_link_filter.rb

@@ -14,6 +14,8 @@ module Banzai
       def call
         return doc unless linkable_files?
 
+        @uri_types = {}
+
         doc.search('a:not(.gfm)').each do |el|
           process_link_attr el.attribute('href')
         end
@@ -48,7 +50,7 @@ module Banzai
         uri.path = [
           relative_url_root,
           context[:project].path_with_namespace,
-          path_type(file_path),
+          uri_type(file_path),
           ref || context[:project].default_branch,  # if no ref exists, point to the default branch
           file_path
         ].compact.join('/').squeeze('/').chomp('/')
@@ -87,7 +89,7 @@ module Banzai
         return path unless request_path
 
         parts = request_path.split('/')
-        parts.pop if path_type(request_path) != 'tree'
+        parts.pop if uri_type(request_path) != :tree
 
         while path.start_with?('../')
           parts.pop
@@ -98,45 +100,20 @@ module Banzai
       end
 
       def file_exists?(path)
-        return false if path.nil?
-        repository.blob_at(current_sha, path).present? ||
-          repository.tree(current_sha, path).entries.any?
-      end
-
-      # Get the type of the given path
-      #
-      # path - String path to check
-      #
-      # Examples:
-      #
-      #   path_type('doc/README.md') # => 'blob'
-      #   path_type('doc/logo.png')  # => 'raw'
-      #   path_type('doc/api')       # => 'tree'
-      #
-      # Returns a String
-      def path_type(path)
-        unescaped_path = Addressable::URI.unescape(path)
-
-        if tree?(unescaped_path)
-          'tree'
-        elsif image?(unescaped_path)
-          'raw'
-        else
-          'blob'
-        end
+        path.present? && !!uri_type(path)
       end
 
-      def tree?(path)
-        repository.tree(current_sha, path).entries.any?
-      end
+      def uri_type(path)
+        @uri_types[path] ||= begin
+          unescaped_path = Addressable::URI.unescape(path)
 
-      def image?(path)
-        repository.blob_at(current_sha, path).try(:image?)
+          current_commit.uri_type(unescaped_path)
+        end
       end
 
-      def current_sha
-        context[:commit].try(:id) ||
-          ref ? repository.commit(ref).try(:sha) : repository.head_commit.sha
+      def current_commit
+        @current_commit ||= context[:commit] ||
+          ref ? repository.commit(ref) : repository.head_commit
       end
 
       def relative_url_root
@@ -148,7 +125,7 @@ module Banzai
       end
 
       def repository
-        context[:project].try(:repository)
+        @repository ||= context[:project].try(:repository)
       end
     end
   end

spec/features/environments_spec.rb

@@ -20,7 +20,7 @@ feature 'Environments', feature: true do
 
     context 'without environments' do
       scenario 'does show no environments' do
-        expect(page).to have_content('No environments to show')
+        expect(page).to have_content('You don\'t have any environments right now.')
       end
     end
 
@@ -61,7 +61,7 @@ feature 'Environments', feature: true do
 
     context 'without deployments' do
       scenario 'does show no deployments' do
-        expect(page).to have_content('No deployments for')
+        expect(page).to have_content('You don\'t have any deployments right now.')
       end
     end
 
@@ -108,7 +108,7 @@ feature 'Environments', feature: true do
         end
 
         scenario 'does create a new pipeline' do
-          expect(page).to have_content('production')
+          expect(page).to have_content('Production')
         end
       end
 

spec/lib/banzai/filter/relative_link_filter_spec.rb

@@ -132,11 +132,8 @@ describe Banzai::Filter::RelativeLinkFilter, lib: true do
       path = 'files/images/한글.png'
       escaped = Addressable::URI.escape(path)
 
-      # Stub these methods so the file doesn't actually need to be in the repo
-      allow_any_instance_of(described_class).
-        to receive(:file_exists?).and_return(true)
-      allow_any_instance_of(described_class).
-        to receive(:image?).with(path).and_return(true)
+      # Stub this method so the file doesn't actually need to be in the repo
+      allow_any_instance_of(described_class).to receive(:uri_type).and_return(:raw)
 
       doc = filter(image(escaped))
       expect(doc.at_css('img')['src']).to match '/raw/'

spec/models/build_spec.rb

@@ -2,7 +2,12 @@ require 'spec_helper'
 
 describe Ci::Build, models: true do
   let(:project) { create(:project) }
-  let(:pipeline) { create(:ci_pipeline, project: project) }
+
+  let(:pipeline) do
+    create(:ci_pipeline, project: project,
+                         sha: project.commit.id)
+  end
+
   let(:build) { create(:ci_build, pipeline: pipeline) }
 
   it { is_expected.to validate_presence_of :ref }
@@ -658,4 +663,10 @@ describe Ci::Build, models: true do
       end
     end
   end
+
+  describe '#commit' do
+    it 'returns commit pipeline has been created for' do
+      expect(build.commit).to eq project.commit
+    end
+  end
 end

spec/models/commit_spec.rb

@@ -207,4 +207,16 @@ eos
       expect(commit.participants).to include(note1.author, note2.author)
     end
   end
+
+  describe '#uri_type' do
+    it 'returns the URI type at the given path' do
+      expect(commit.uri_type('files/html')).to be(:tree)
+      expect(commit.uri_type('files/images/logo-black.png')).to be(:raw)
+      expect(commit.uri_type('files/js/application.js')).to be(:blob)
+    end
+
+    it "returns nil if the path doesn't exists" do
+      expect(commit.uri_type('this/path/doesnt/exist')).to be_nil
+    end
+  end
 end

spec/models/commit_status_spec.rb

 require 'spec_helper'
 
 describe CommitStatus, models: true do
-  let(:pipeline) { FactoryGirl.create :ci_pipeline }
-  let(:commit_status) { FactoryGirl.create :commit_status, pipeline: pipeline }
+  let(:project) { create(:project) }
+
+  let(:pipeline) do
+    create(:ci_pipeline, project: project, sha: project.commit.id)
+  end
+
+  let(:commit_status) { create(:commit_status, pipeline: pipeline) }
 
   it { is_expected.to belong_to(:pipeline) }
   it { is_expected.to belong_to(:user) }
@@ -13,7 +18,7 @@ describe CommitStatus, models: true do
 
   it { is_expected.to delegate_method(:sha).to(:pipeline) }
   it { is_expected.to delegate_method(:short_sha).to(:pipeline) }
-  
+
   it { is_expected.to respond_to :success? }
   it { is_expected.to respond_to :failed? }
   it { is_expected.to respond_to :running? }
@@ -116,7 +121,7 @@ describe CommitStatus, models: true do
       it { is_expected.to be > 0.0 }
     end
   end
-  
+
   describe :latest do
     subject { CommitStatus.latest.order(:id) }
 
@@ -198,4 +203,10 @@ describe CommitStatus, models: true do
       end
     end
   end
+
+  describe '#commit' do
+    it 'returns commit pipeline has been created for' do
+      expect(commit_status.commit).to eq project.commit
+    end
+  end
 end

spec/requests/api/builds_spec.rb

@@ -9,8 +9,8 @@ describe API::API, api: true  do
   let!(:project) { create(:project, creator_id: user.id) }
   let!(:developer) { create(:project_member, :developer, user: user, project: project) }
   let!(:reporter) { create(:project_member, :reporter, user: user2, project: project) }
-  let(:pipeline) { create(:ci_pipeline, project: project)}
-  let(:build) { create(:ci_build, pipeline: pipeline) }
+  let!(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.id) }
+  let!(:build) { create(:ci_build, pipeline: pipeline) }
 
   describe 'GET /projects/:id/builds ' do
     let(:query) { '' }
@@ -23,6 +23,11 @@ describe API::API, api: true  do
         expect(json_response).to be_an Array
       end
 
+      it 'returns correct values' do
+        expect(json_response).not_to be_empty
+        expect(json_response.first['commit']['id']).to eq project.commit.id
+      end
+
       context 'filter project with one scope element' do
         let(:query) { 'scope=pending' }
 
@@ -132,7 +137,7 @@ describe API::API, api: true  do
 
   describe 'GET /projects/:id/builds/:build_id/trace' do
     let(:build) { create(:ci_build, :trace, pipeline: pipeline) }
-    
+
     before { get api("/projects/#{project.id}/builds/#{build.id}/trace", api_user) }
 
     context 'authorized user' do