diff --git a/CHANGELOG b/CHANGELOG index 5abaff41e95..0c3aadc29d1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ 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 diff --git a/app/assets/javascripts/application.js.coffee b/app/assets/javascripts/application.js.coffee index 4529c514555..397892d15c0 100644 --- a/app/assets/javascripts/application.js.coffee +++ b/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') diff --git a/app/assets/javascripts/notifications_dropdown.js.coffee b/app/assets/javascripts/notifications_dropdown.js.coffee index 74d2298c1fa..0bbd082c156 100644 --- a/app/assets/javascripts/notifications_dropdown.js.coffee +++ b/app/assets/javascripts/notifications_dropdown.js.coffee @@ -1,5 +1,5 @@ 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') diff --git a/app/assets/javascripts/project.js.coffee b/app/assets/javascripts/project.js.coffee index 54c539d5f9b..96e10dd7e8a 100644 --- a/app/assets/javascripts/project.js.coffee +++ b/app/assets/javascripts/project.js.coffee @@ -35,7 +35,6 @@ class @Project $(@).parents('.no-password-message').remove() e.preventDefault() - @projectSelectDropdown() projectSelectDropdown: -> diff --git a/app/assets/stylesheets/framework.scss b/app/assets/stylesheets/framework.scss index 3cbddc59f11..a306b8f3f29 100644 --- a/app/assets/stylesheets/framework.scss +++ b/app/assets/stylesheets/framework.scss @@ -37,3 +37,4 @@ @import "framework/timeline.scss"; @import "framework/typography.scss"; @import "framework/zen.scss"; +@import "framework/blank"; diff --git a/app/assets/stylesheets/framework/blank.scss b/app/assets/stylesheets/framework/blank.scss new file mode 100644 index 00000000000..40b5171a8c6 --- /dev/null +++ b/app/assets/stylesheets/framework/blank.scss @@ -0,0 +1,23 @@ +.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; +} diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 346badf6d86..d3e59d7fdb9 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -101,7 +101,8 @@ .notifications-btn { - .fa-bell { + .fa-bell, + .fa-spinner { margin-right: 6px; } diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 127bd1a4318..487963fdcd7 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/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 diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 5b264ecffc5..ca5a685dd11 100644 --- a/app/models/ci/pipeline.rb +++ b/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 diff --git a/app/models/commit.rb b/app/models/commit.rb index d69d518fadd..174ccbaea6c 100644 --- a/app/models/commit.rb +++ b/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 diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index ab13db4b297..e437e3417a8 100644 --- a/app/models/commit_status.rb +++ b/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 diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index 199ab3c38c3..2234bf79c87 100644 --- a/app/views/layouts/_page.html.haml +++ b/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 diff --git a/app/views/profiles/notifications/show.html.haml b/app/views/profiles/notifications/show.html.haml index 5afd83a522e..f77738f97f5 100644 --- a/app/views/profiles/notifications/show.html.haml +++ b/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 diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index b8d8758fd2b..e38d1ff5ff0 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/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 diff --git a/app/views/projects/environments/index.html.haml b/app/views/projects/environments/index.html.haml index ae9e77e7d89..a03f117291f 100644 --- a/app/views/projects/environments/index.html.haml +++ b/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 diff --git a/app/views/projects/environments/new.html.haml b/app/views/projects/environments/new.html.haml index 54465828ba9..da325efecd2 100644 --- a/app/views/projects/environments/new.html.haml +++ b/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' diff --git a/app/views/projects/environments/show.html.haml b/app/views/projects/environments/show.html.haml index 069b77b5adf..4c15e2759d6 100644 --- a/app/views/projects/environments/show.html.haml +++ b/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 diff --git a/features/steps/profile/notifications.rb b/features/steps/profile/notifications.rb index 979f4692d5a..7e339443b75 100644 --- a/features/steps/profile/notifications.rb +++ b/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 diff --git a/features/steps/project/project.rb b/features/steps/project/project.rb index 98b57e5cbfb..76fefee9254 100644 --- a/features/steps/project/project.rb +++ b/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 diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 0ee96d4c67b..5a23a18fe9c 100644 --- a/lib/api/entities.rb +++ b/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 diff --git a/lib/banzai/filter/relative_link_filter.rb b/lib/banzai/filter/relative_link_filter.rb index ea21c7b041c..c78da404607 100644 --- a/lib/banzai/filter/relative_link_filter.rb +++ b/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? + path.present? && !!uri_type(path) 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) + def uri_type(path) + @uri_types[path] ||= begin + unescaped_path = Addressable::URI.unescape(path) - if tree?(unescaped_path) - 'tree' - elsif image?(unescaped_path) - 'raw' - else - 'blob' + current_commit.uri_type(unescaped_path) end end - def tree?(path) - repository.tree(current_sha, path).entries.any? - end - - def image?(path) - repository.blob_at(current_sha, path).try(:image?) - 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 diff --git a/spec/features/environments_spec.rb b/spec/features/environments_spec.rb index 40fea5211e9..7fb28f4174b 100644 --- a/spec/features/environments_spec.rb +++ b/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 diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb index 0e6685f0ffb..b9e4a4eaf0e 100644 --- a/spec/lib/banzai/filter/relative_link_filter_spec.rb +++ b/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/' diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 479b7309579..8154001cf46 100644 --- a/spec/models/build_spec.rb +++ b/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 diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index beca8708c9d..ba02d5fe977 100644 --- a/spec/models/commit_spec.rb +++ b/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 diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 8fb605fff8a..96397d7c8a9 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -1,8 +1,13 @@ 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 diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index ac85f340922..47e9253a10c 100644 --- a/spec/requests/api/builds_spec.rb +++ b/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