diff options
author | Lin Jen-Shin <godfat@godfat.org> | 2017-03-20 22:28:38 +0800 |
---|---|---|
committer | Lin Jen-Shin <godfat@godfat.org> | 2017-03-20 22:28:38 +0800 |
commit | 6a6fd757608402014e844857f70b003123fec0e8 (patch) | |
tree | 00e67fe5e7f428659de9d232f21ecf0af9bba5da | |
parent | 67f4501f5f240974fa9bd4eba3e7e08cb1a63bf4 (diff) | |
parent | bb3fea8919ce62461f44ae9e679277c7144e9a5f (diff) | |
download | gitlab-ce-6a6fd757608402014e844857f70b003123fec0e8.tar.gz |
Merge branch '9-0-stable' into 9-0-stable-dev
* 9-0-stable:
Merge branch 'protobuf-3.2.0.2' into 'master'
Merge branch '28890-allow-creating-mr-without-target-branch-in-url' into 'master'
Merge branch '24683-sidebar-spinners' into 'master'
Merge branch '29534-todos-performance' into 'master'
Merge branch '29586-pipelines-tabs' into 'master'
Merge branch 'fix/regressions-in-ci-v4-api' into 'master'
23 files changed, 173 insertions, 53 deletions
diff --git a/Gemfile.lock b/Gemfile.lock index c60c045a4c2..fd44d4973ab 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -304,7 +304,7 @@ GEM multi_json (~> 1.10) retriable (~> 1.4) signet (~> 0.6) - google-protobuf (3.2.0) + google-protobuf (3.2.0.2) googleauth (0.5.1) faraday (~> 0.9) jwt (~> 1.4) diff --git a/app/assets/javascripts/due_date_select.js b/app/assets/javascripts/due_date_select.js index 9169fcd7328..b92694cfeb5 100644 --- a/app/assets/javascripts/due_date_select.js +++ b/app/assets/javascripts/due_date_select.js @@ -133,7 +133,7 @@ const selectedDateValue = this.datePayload[this.abilityName].due_date; const displayedDateStyle = this.displayedDate !== 'No due date' ? 'bold' : 'no-value'; - this.$loading.fadeIn(); + this.$loading.removeClass('hidden').fadeIn(); if (isDropdown) { this.$dropdown.trigger('loading.gl.dropdown'); diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 9e2d14c7f87..2bce2e2a547 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -76,7 +76,7 @@ if (!selected.length) { data[abilityName].label_ids = ['']; } - $loading.fadeIn(); + $loading.removeClass('hidden').fadeIn(); $dropdown.trigger('loading.gl.dropdown'); return $.ajax({ type: 'PUT', diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js index 51fa5c828b3..5b25d72244c 100644 --- a/app/assets/javascripts/milestone_select.js +++ b/app/assets/javascripts/milestone_select.js @@ -157,7 +157,7 @@ } $dropdown.trigger('loading.gl.dropdown'); - $loading.fadeIn(); + $loading.removeClass('hidden').fadeIn(); gl.issueBoards.BoardsStore.detail.issue.update($dropdown.attr('data-issue-update')) .then(function () { @@ -169,7 +169,7 @@ data = {}; data[abilityName] = {}; data[abilityName].milestone_id = selected != null ? selected : null; - $loading.fadeIn(); + $loading.removeClass('hidden').fadeIn(); $dropdown.trigger('loading.gl.dropdown'); return $.ajax({ type: 'PUT', diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js index 27af859f7d8..910af8bf9f9 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -53,7 +53,7 @@ $loading = $block.find('.block-loading').fadeOut(); var updateIssueBoardsIssue = function () { - $loading.fadeIn(); + $loading.removeClass('hidden').fadeIn(); gl.issueBoards.BoardsStore.detail.issue.update($dropdown.attr('data-issue-update')) .then(function () { $loading.fadeOut(); @@ -90,7 +90,7 @@ data = {}; data[abilityName] = {}; data[abilityName].assignee_id = selected != null ? selected : null; - $loading.fadeIn(); + $loading.removeClass('hidden').fadeIn(); $dropdown.trigger('loading.gl.dropdown'); return $.ajax({ type: 'PUT', diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index ea45aaa0253..205d23b1329 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -138,7 +138,6 @@ .nav-links { display: inline-block; - width: 50%; margin-bottom: 0; border-bottom: none; diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index 5848ca62777..00576272b62 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -45,7 +45,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController private def find_todos - @todos ||= TodosFinder.new(current_user, params).execute + @todos ||= TodosFinder.new(current_user, params.merge(include_associations: true)).execute end def todos_counts diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index b7f091f334d..13d33a1c31b 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -24,6 +24,7 @@ class TodosFinder def execute items = current_user.todos + items = include_associations(items) items = by_action_id(items) items = by_action(items) items = by_author(items) @@ -38,6 +39,17 @@ class TodosFinder private + def include_associations(items) + return items unless params[:include_associations] + + items.includes( + [ + target: { project: [:route, namespace: :route] }, + author: { namespace: :route }, + ] + ) + end + def action_id? action_id.present? && Todo::ACTION_NAMES.has_key?(action_id.to_i) end diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index 4f5adf623f2..847a8fdfca6 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -39,9 +39,13 @@ module TodosHelper namespace_project_commit_path(todo.project.namespace.becomes(Namespace), todo.project, todo.target, anchor: anchor) else - path = [todo.project.namespace.becomes(Namespace), todo.project, todo.target] - - path.unshift(:pipelines) if todo.build_failed? + if todo.build_failed? + # associated namespace and route would be loaded from the db again if todo.project was used + project = todo.target.project + path = [:pipelines, project.namespace.becomes(Namespace), project, todo.target] + else + path = [todo.target] + end polymorphic_path(path, anchor: anchor) end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 53fc0d87823..ad0be70c32a 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -539,6 +539,16 @@ module Ci Gitlab::Ci::Build::Credentials::Factory.new(self).create! end + def dependencies + depended_jobs = depends_on_builds + + return depended_jobs unless options[:dependencies].present? + + depended_jobs.select do |job| + options[:dependencies].include?(job.name) + end + end + private def update_artifacts_size diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 0f7b8311588..d782f7847d7 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -7,6 +7,7 @@ class MergeRequest < ActiveRecord::Base belongs_to :target_project, class_name: "Project" belongs_to :source_project, class_name: "Project" + belongs_to :project, foreign_key: :target_project_id belongs_to :merge_user, class_name: "User" has_many :merge_request_diffs, dependent: :destroy @@ -537,10 +538,6 @@ class MergeRequest < ActiveRecord::Base target_project != source_project end - def project - target_project - end - # If the merge request closes any issues, save this information in the # `MergeRequestsClosingIssues` model. This is a performance optimization. # Calculating this information for a number of merge requests requires diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 9d4739e37bb..fdce542bd9e 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -6,7 +6,7 @@ module MergeRequests merge_request.source_project = find_source_project merge_request.target_project = find_target_project merge_request.target_branch = find_target_branch - merge_request.can_be_created = branches_valid? && source_branch_specified? && target_branch_specified? + merge_request.can_be_created = branches_valid? compare_branches if branches_present? assign_title_and_description if merge_request.can_be_created diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 048fc488207..25a4aec0a38 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -30,7 +30,7 @@ = icon('user', 'aria-hidden': 'true') .title.hide-collapsed Assignee - = icon('spinner spin', class: 'block-loading', 'aria-hidden': 'true') + = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can_edit_issuable = link_to 'Edit', '#', class: 'edit-link pull-right' .value.hide-collapsed @@ -64,7 +64,7 @@ None .title.hide-collapsed Milestone - = icon('spinner spin', class: 'block-loading', 'aria-hidden': 'true') + = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can_edit_issuable = link_to 'Edit', '#', class: 'edit-link pull-right' .value.hide-collapsed @@ -91,7 +91,7 @@ = issuable.due_date.try(:to_s, :medium) || 'None' .title.hide-collapsed Due date - = icon('spinner spin', class: 'block-loading', 'aria-hidden': 'true') + = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can?(current_user, :"admin_#{issuable.to_ability_name}", @project) = link_to 'Edit', '#', class: 'edit-link pull-right' .value.hide-collapsed @@ -121,12 +121,12 @@ - selected_labels = issuable.labels .block.labels .sidebar-collapsed-icon.js-sidebar-labels-tooltip{ title: issuable_labels_tooltip(issuable.labels_array), data: { placement: "left", container: "body" } } - = icon('tags', 'aria-hidden': 'true') + = icon('tags', class: 'hidden', 'aria-hidden': 'true') %span = selected_labels.size .title.hide-collapsed Labels - = icon('spinner spin', class: 'block-loading', 'aria-hidden': 'true') + = icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true') - if can_edit_issuable = link_to 'Edit', '#', class: 'edit-link pull-right' .value.issuable-show-labels.hide-collapsed{ class: ("has-labels" if selected_labels.any?) } diff --git a/changelogs/unreleased/24683-sidebar-spinners.yml b/changelogs/unreleased/24683-sidebar-spinners.yml new file mode 100644 index 00000000000..3fec273152f --- /dev/null +++ b/changelogs/unreleased/24683-sidebar-spinners.yml @@ -0,0 +1,4 @@ +--- +title: hide loading spinners for server-rendered sidebar fields +merge_request: +author: diff --git a/changelogs/unreleased/28890-allow-creating-mr-without-target-branch-in-url.yml b/changelogs/unreleased/28890-allow-creating-mr-without-target-branch-in-url.yml new file mode 100644 index 00000000000..114a14ec2df --- /dev/null +++ b/changelogs/unreleased/28890-allow-creating-mr-without-target-branch-in-url.yml @@ -0,0 +1,5 @@ +--- +title: Allow creating merge request even if target branch is not specified in query + params +merge_request: 9968 +author: diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 0a12ee72d49..5954aea8041 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -768,7 +768,7 @@ module API end class Dependency < Grape::Entity - expose :id, :name + expose :id, :name, :token expose :artifacts_file, using: ArtifactFile, if: ->(job, _) { job.artifacts? } end @@ -796,7 +796,7 @@ module API expose :artifacts, using: Artifacts expose :cache, using: Cache expose :credentials, using: Credentials - expose :depends_on_builds, as: :dependencies, using: Dependency + expose :dependencies, using: Dependency end end end diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb index ec2bcaed929..74848a6e144 100644 --- a/lib/api/helpers/runner.rb +++ b/lib/api/helpers/runner.rb @@ -41,14 +41,6 @@ module API (Time.now - current_runner.contacted_at) >= contacted_at_max_age end - def job_not_found! - if headers['User-Agent'].to_s =~ /gitlab(-ci-multi)?-runner \d+\.\d+\.\d+(~beta\.\d+\.g[0-9a-f]+)? / - no_content! - else - not_found! - end - end - def validate_job!(job) not_found! unless job diff --git a/lib/api/runner.rb b/lib/api/runner.rb index c700d2ef4a1..4c9db2c8716 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -47,11 +47,25 @@ module API authenticate_runner! Ci::Runner.find_by_token(params[:token]).destroy end + + desc 'Validates authentication credentials' do + http_codes [[200, 'Credentials are valid'], [403, 'Forbidden']] + end + params do + requires :token, type: String, desc: %q(Runner's authentication token) + end + post '/verify' do + authenticate_runner! + status 200 + end end resource :jobs do desc 'Request a job' do success Entities::JobRequest::Response + http_codes [[201, 'Job was scheduled'], + [204, 'No job for Runner'], + [403, 'Forbidden']] end params do requires :token, type: String, desc: %q(Runner's authentication token) @@ -60,13 +74,13 @@ module API end post '/request' do authenticate_runner! - not_found! unless current_runner.active? + no_content! unless current_runner.active? update_runner_info if current_runner.is_runner_queue_value_latest?(params[:last_update]) header 'X-GitLab-Last-Update', params[:last_update] Gitlab::Metrics.add_event(:build_not_found_cached) - return job_not_found! + return no_content! end new_update = current_runner.ensure_runner_queue_value @@ -80,7 +94,7 @@ module API else Gitlab::Metrics.add_event(:build_not_found) header 'X-GitLab-Last-Update', new_update - job_not_found! + no_content! end else # We received build that is invalid due to concurrency conflict diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index ae0bbbd6aeb..21487541507 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -4,6 +4,7 @@ FactoryGirl.define do author association :source_project, :repository, factory: :project target_project { source_project } + project { target_project } # $ git log --pretty=oneline feature..master # 5937ac0a7beb003549fc5fd26fc247adbce4a52e Add submodule from gitlab.com diff --git a/spec/features/merge_requests/user_uses_slash_commands_spec.rb b/spec/features/merge_requests/user_uses_slash_commands_spec.rb index 2f3c3e45ae6..a1f4eb2688b 100644 --- a/spec/features/merge_requests/user_uses_slash_commands_spec.rb +++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb @@ -133,7 +133,6 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do it 'changes target_branch in new merge_request' do visit new_namespace_project_merge_request_path(another_project.namespace, another_project, new_url_opts) - click_button "Compare branches and continue" fill_in "merge_request_title", with: 'My brand new feature' fill_in "merge_request_description", with: "le feature \n/target_branch fix\nFeature description:" diff --git a/spec/helpers/todos_helper_spec.rb b/spec/helpers/todos_helper_spec.rb index 50060a0925d..21e0e74e008 100644 --- a/spec/helpers/todos_helper_spec.rb +++ b/spec/helpers/todos_helper_spec.rb @@ -1,6 +1,40 @@ require "spec_helper" describe TodosHelper do + include GitlabRoutingHelper + + describe '#todo_target_path' do + let(:project) { create(:project) } + let(:merge_request) { create(:merge_request, target_project: project, source_project: project) } + let(:issue) { create(:issue, project: project) } + let(:note) { create(:note_on_issue, noteable: issue, project: project) } + + let(:mr_todo) { build(:todo, project: project, target: merge_request) } + let(:issue_todo) { build(:todo, project: project, target: issue) } + let(:note_todo) { build(:todo, project: project, target: issue, note: note) } + let(:build_failed_todo) { build(:todo, :build_failed, project: project, target: merge_request) } + + it 'returns correct path to the todo MR' do + expect(todo_target_path(mr_todo)). + to eq("/#{project.full_path}/merge_requests/#{merge_request.iid}") + end + + it 'returns correct path to the todo issue' do + expect(todo_target_path(issue_todo)). + to eq("/#{project.full_path}/issues/#{issue.iid}") + end + + it 'returns correct path to the todo note' do + expect(todo_target_path(note_todo)). + to eq("/#{project.full_path}/issues/#{issue.iid}#note_#{note.id}") + end + + it 'returns correct path to build_todo MR when pipeline failed' do + expect(todo_target_path(build_failed_todo)). + to eq("/#{project.full_path}/merge_requests/#{merge_request.iid}/pipelines") + end + end + describe '#todo_projects_options' do let(:projects) { create_list(:empty_project, 3) } let(:user) { create(:user) } diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 442b2df1952..044b989e5ba 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -152,6 +152,34 @@ describe API::Runner do end end end + + describe 'POST /api/v4/runners/verify' do + let(:runner) { create(:ci_runner) } + + context 'when no token is provided' do + it 'returns 400 error' do + post api('/runners/verify') + + expect(response).to have_http_status :bad_request + end + end + + context 'when invalid token is provided' do + it 'returns 403 error' do + post api('/runners/verify'), token: 'invalid-token' + + expect(response).to have_http_status 403 + end + end + + context 'when valid token is provided' do + it 'verifies Runner credentials' do + post api('/runners/verify'), token: runner.token + + expect(response).to have_http_status 200 + end + end + end end describe '/api/v4/jobs' do @@ -220,18 +248,6 @@ describe API::Runner do it { expect(response).to have_http_status(204) } end end - - context "when runner doesn't send version in User-Agent" do - let(:user_agent) { 'Go-http-client/1.1' } - - it { expect(response).to have_http_status(404) } - end - - context "when runner doesn't have a User-Agent" do - let(:user_agent) { nil } - - it { expect(response).to have_http_status(404) } - end end context 'when no token is provided' do @@ -254,10 +270,10 @@ describe API::Runner do context 'when Runner is not active' do let(:runner) { create(:ci_runner, :inactive) } - it 'returns 404 error' do + it 'returns 204 error' do request_job - expect(response).to have_http_status 404 + expect(response).to have_http_status 204 end end @@ -401,9 +417,39 @@ describe API::Runner do end context 'when project and pipeline have multiple jobs' do + let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + let!(:job2) { create(:ci_build_tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) } let!(:test_job) { create(:ci_build, pipeline: pipeline, name: 'deploy', stage: 'deploy', stage_idx: 1) } - before { job.success } + before do + job.success + job2.success + end + + it 'returns dependent jobs' do + request_job + + expect(response).to have_http_status(201) + expect(json_response['id']).to eq(test_job.id) + expect(json_response['dependencies'].count).to eq(2) + expect(json_response['dependencies']).to include({ 'id' => job.id, 'name' => job.name, 'token' => job.token }, + { 'id' => job2.id, 'name' => job2.name, 'token' => job2.token }) + end + end + + context 'when explicit dependencies are defined' do + let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + let!(:job2) { create(:ci_build_tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) } + let!(:test_job) do + create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'deploy', + stage: 'deploy', stage_idx: 1, + options: { dependencies: [job2.name] }) + end + + before do + job.success + job2.success + end it 'returns dependent jobs' do request_job @@ -411,7 +457,7 @@ describe API::Runner do expect(response).to have_http_status(201) expect(json_response['id']).to eq(test_job.id) expect(json_response['dependencies'].count).to eq(1) - expect(json_response['dependencies'][0]).to include('id' => job.id, 'name' => 'spinach') + expect(json_response['dependencies'][0]).to include('id' => job2.id, 'name' => job2.name, 'token' => job2.token) end end diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index 0768f644036..adfa75a524f 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -49,10 +49,13 @@ describe MergeRequests::BuildService, services: true do let(:commits) { Commit.decorate([commit_1], project) } it 'creates compare object with target branch as default branch' do - expect(merge_request.can_be_created).to eq(false) expect(merge_request.compare).to be_present expect(merge_request.target_branch).to eq(project.default_branch) end + + it 'allows the merge request to be created' do + expect(merge_request.can_be_created).to eq(true) + end end context 'same source and target branch' do |