diff options
44 files changed, 572 insertions, 115 deletions
diff --git a/CHANGELOG b/CHANGELOG index b00c149a753..0c712b445a4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -31,6 +31,7 @@ v 8.9.0 (unreleased) - Add rake task 'gitlab:db:configure' for conditionally seeding or migrating the database - Changed the Slack build message to use the singular duration if necessary (Aran Koning) - Links from a wiki page to other wiki pages should be rewritten as expected + - Add option to project to only allow merge requests to be merged if the build succeeds (Rui Santos) - Fix issues filter when ordering by milestone - Todos will display target state if issuable target is 'Closed' or 'Merged' - Fix bug when sorting issues by milestone due date and filtering by two or more labels @@ -61,6 +62,7 @@ v 8.9.0 (unreleased) - Markdown editor now correctly resets the input value on edit cancellation !4175 - Toggling a task list item in a issue/mr description does not creates a Todo for mentions - Improved UX of date pickers on issue & milestone forms + - Cache on the database if a project has an active external issue tracker. v 8.8.5 (unreleased) - Ensure branch cleanup regardless of whether the GitHub import process succeeds @@ -70,6 +72,7 @@ v 8.8.5 (unreleased) - Import GitHub repositories respecting the API rate limit - Fix importer for GitHub comments on diff - Disable Webhooks before proceeding with the GitHub import + - Fix incremental trace upload API when using multi-byte UTF-8 chars in trace v 8.8.4 - Fix LDAP-based login for users with 2FA enabled. !4493 diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index ec74dfaae1a..9ca88f1226e 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -95,8 +95,11 @@ class @LabelsSelect $newLabelCreateButton.enable() if label.message? + errors = _.map label.message, (value, key) -> + "#{key} #{value[0]}" + $newLabelError - .text label.message + .html errors.join("<br/>") .show() else $('.dropdown-menu-back', $dropdown.parent()).trigger 'click' @@ -254,7 +257,7 @@ class @LabelsSelect search: fields: ['title'] selectable: true - + filterable: true toggleLabel: (selected, el) -> selected_labels = $('.js-label-select').siblings('.dropdown-menu-labels').find('.is-active') diff --git a/app/assets/javascripts/project_new.js.coffee b/app/assets/javascripts/project_new.js.coffee index 63dee4ed5d7..e48343a19b5 100644 --- a/app/assets/javascripts/project_new.js.coffee +++ b/app/assets/javascripts/project_new.js.coffee @@ -7,12 +7,17 @@ class @ProjectNew @toggleSettingsOnclick() - toggleSettings: -> - checked = $("#project_builds_enabled").prop("checked") - if checked - $('.builds-feature').show() - else - $('.builds-feature').hide() + toggleSettings: => + @_showOrHide('#project_builds_enabled', '.builds-feature') + @_showOrHide('#project_merge_requests_enabled', '.merge-requests-feature') toggleSettingsOnclick: -> - $("#project_builds_enabled").on 'click', @toggleSettings + $('#project_builds_enabled, #project_merge_requests_enabled').on 'click', @toggleSettings + + _showOrHide: (checkElement, container) -> + $container = $(container) + + if $(checkElement).prop('checked') + $container.show() + else + $container.hide() diff --git a/app/assets/javascripts/users_select.js.coffee b/app/assets/javascripts/users_select.js.coffee index de0eae58bff..88246b0feb8 100644 --- a/app/assets/javascripts/users_select.js.coffee +++ b/app/assets/javascripts/users_select.js.coffee @@ -95,7 +95,7 @@ class @UsersSelect data: (term, callback) => isAuthorFilter = $('.js-author-search') - @users term, term is '' and isAuthorFilter, (users) => + @users term, (users) => if term.length is 0 showDivider = 0 @@ -221,7 +221,7 @@ class @UsersSelect multiple: $(select).hasClass('multiselect') minimumInputLength: 0 query: (query) => - @users query.term, @projectId?, (users) => + @users query.term, (users) => data = { results: users } if query.term.length == 0 @@ -304,7 +304,7 @@ class @UsersSelect # Return users list. Filtered by query # Only active users retrieved - users: (query, fromProject, callback) => + users: (query, callback) => url = @buildUrl(@usersPath) $.ajax( @@ -313,7 +313,7 @@ class @UsersSelect search: query per_page: 20 active: true - project_id: @projectId if fromProject + project_id: @projectId group_id: @groupId current_user: @showCurrentUser author_id: @authorId diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index c46d6b14782..b8d4233537b 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -185,7 +185,7 @@ header { margin-left: 0; .header-content { - padding-left: $sidebar_width; + margin-left: $sidebar_width; transition-duration: .3s; } } diff --git a/app/assets/stylesheets/pages/awards.scss b/app/assets/stylesheets/pages/awards.scss index 05d1ee5b998..6211f3a52eb 100644 --- a/app/assets/stylesheets/pages/awards.scss +++ b/app/assets/stylesheets/pages/awards.scss @@ -101,13 +101,21 @@ line-height: 20px; outline: 0; + &:hover, &.active, &:active { - background-color: $white-dark; + background-color: $row-hover; + border-color: $row-hover-border; box-shadow: none; outline: 0; } + &.btn { + &:focus { + outline: 0; + } + } + &.is-loading { .award-control-icon-normal, .emoji-icon { diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 3af62c7696c..a6479c42d94 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -234,7 +234,7 @@ class ProjectsController < Projects::ApplicationController :issues_tracker_id, :default_branch, :wiki_enabled, :visibility_level, :import_url, :last_activity_at, :namespace_id, :avatar, :builds_enabled, :build_allow_git_fetch, :build_timeout_in_minutes, :build_coverage_regex, - :public_builds, + :public_builds, :only_allow_merge_if_build_succeeds ) end diff --git a/app/helpers/dropdowns_helper.rb b/app/helpers/dropdowns_helper.rb index 14697f774cc..6b617e1730a 100644 --- a/app/helpers/dropdowns_helper.rb +++ b/app/helpers/dropdowns_helper.rb @@ -67,9 +67,9 @@ module DropdownsHelper end end - def dropdown_filter(placeholder) + def dropdown_filter(placeholder, search_id: nil) content_tag :div, class: "dropdown-input" do - filter_output = search_field_tag nil, nil, class: "dropdown-input-field", placeholder: placeholder + filter_output = search_field_tag search_id, nil, class: "dropdown-input-field", placeholder: placeholder filter_output << icon('search', class: "dropdown-input-search") filter_output << icon('times', class: "dropdown-input-clear js-dropdown-input-clear", role: "button") diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index b8ada6361ac..6a64ca451f7 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -194,7 +194,7 @@ module Ci def trace_length if raw_trace - raw_trace.length + raw_trace.bytesize else 0 end @@ -216,7 +216,7 @@ module Ci recreate_trace_dir File.truncate(path_to_trace, offset) if File.exist?(path_to_trace) - File.open(path_to_trace, 'a') do |f| + File.open(path_to_trace, 'ab') do |f| f.write(trace_part) end end diff --git a/app/models/ci/trigger_request.rb b/app/models/ci/trigger_request.rb index 59fc9951d11..b69ae37668c 100644 --- a/app/models/ci/trigger_request.rb +++ b/app/models/ci/trigger_request.rb @@ -3,7 +3,7 @@ module Ci extend Ci::Model belongs_to :trigger, class_name: 'Ci::Trigger' - belongs_to :commit, class_name: 'Ci::Pipeline', foreign_key: :commit_id + belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id has_many :builds, class_name: 'Ci::Build' serialize :variables diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b0ed8182855..29f36570912 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -260,13 +260,22 @@ class MergeRequest < ActiveRecord::Base end def mergeable? - return false unless open? && !work_in_progress? && !broken? + return false unless mergeable_state? check_if_can_be_merged can_be_merged? end + def mergeable_state? + return false unless open? + return false if work_in_progress? + return false if broken? + return false unless mergeable_ci_state? + + true + end + def gitlab_merge_status if work_in_progress? "work_in_progress" @@ -481,6 +490,12 @@ class MergeRequest < ActiveRecord::Base ::Gitlab::GitAccess.new(user, project).can_push_to_branch?(target_branch) end + def mergeable_ci_state? + return true unless project.only_allow_merge_if_build_succeeds? + + !pipeline || pipeline.success? + end + def state_human_name if merged? "Merged" diff --git a/app/models/project.rb b/app/models/project.rb index f47ef8a81de..472e79cfcf8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -523,9 +523,21 @@ class Project < ActiveRecord::Base end def external_issue_tracker - return @external_issue_tracker if defined?(@external_issue_tracker) - @external_issue_tracker ||= - services.issue_trackers.active.without_defaults.first + if has_external_issue_tracker.nil? # To populate existing projects + cache_has_external_issue_tracker + end + + if has_external_issue_tracker? + return @external_issue_tracker if defined?(@external_issue_tracker) + + @external_issue_tracker = services.external_issue_trackers.first + else + nil + end + end + + def cache_has_external_issue_tracker + update_column(:has_external_issue_tracker, services.external_issue_trackers.any?) end def can_have_issues_tracker_id? diff --git a/app/models/service.rb b/app/models/service.rb index de3fd24584a..bf352397509 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -16,6 +16,7 @@ class Service < ActiveRecord::Base after_initialize :initialize_properties after_commit :reset_updated_properties + after_commit :cache_project_has_external_issue_tracker belongs_to :project has_one :service_hook @@ -34,6 +35,7 @@ class Service < ActiveRecord::Base scope :note_hooks, -> { where(note_events: true, active: true) } scope :build_hooks, -> { where(build_events: true, active: true) } scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) } + scope :external_issue_trackers, -> { issue_trackers.active.without_defaults } default_value_for :category, 'common' @@ -192,4 +194,12 @@ class Service < ActiveRecord::Base service.project_id = project_id service if service.save end + + private + + def cache_project_has_external_issue_tracker + if project && !project.destroyed? + project.cache_has_external_issue_tracker + end + end end diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index c3194f45b10..1e629cf119a 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -11,7 +11,7 @@ module Ci trigger_request = trigger.trigger_requests.create!( variables: variables, - commit: pipeline, + pipeline: pipeline, ) if pipeline.create_builds(nil, trigger_request) diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 2a58ef224b3..ca99ba8def3 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -129,5 +129,4 @@ %li.hidden = link_to project_commits_path(@project), title: 'Commits', class: 'shortcuts-commits' do Commits - .fade-right diff --git a/app/views/projects/_merge_request_settings.html.haml b/app/views/projects/_merge_request_settings.html.haml new file mode 100644 index 00000000000..da522b53417 --- /dev/null +++ b/app/views/projects/_merge_request_settings.html.haml @@ -0,0 +1,11 @@ +%fieldset.builds-feature + %h5.prepend-top-0 + Merge Requests + .form-group + .checkbox + = f.label :only_allow_merge_if_build_succeeds do + = f.check_box :only_allow_merge_if_build_succeeds + %strong Only allow merge requests to be merged if the build succeeds + .help-block + Builds need to be configured to enable this feature. + = link_to icon('question-circle'), help_page_path('workflow', 'merge_requests#only-allow-merge-requests-to-be-merged-if-the-build-succeeds') diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 18b125ff9d4..8449fe1e4e0 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -84,6 +84,8 @@ %br %span.descr Enable Container Registry for this repository %hr + = render 'merge_request_settings', f: f + %hr = render 'builds_settings', f: f %hr %fieldset.features.append-bottom-default diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml index 13359abede7..0e0af57d76e 100644 --- a/app/views/projects/merge_requests/widget/_open.html.haml +++ b/app/views/projects/merge_requests/widget/_open.html.haml @@ -17,6 +17,8 @@ = render 'projects/merge_requests/widget/open/merge_when_build_succeeds' - elsif !@merge_request.can_be_merged_by?(current_user) = render 'projects/merge_requests/widget/open/not_allowed' + - elsif !@merge_request.mergeable_ci_state? && @pipeline && @pipeline.failed? + = render 'projects/merge_requests/widget/open/build_failed' - elsif @merge_request.can_be_merged? = render 'projects/merge_requests/widget/open/accept' diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml index 60d7d6ff1f5..941513febbd 100644 --- a/app/views/projects/merge_requests/widget/open/_accept.html.haml +++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml @@ -10,19 +10,20 @@ %span.btn-group = button_tag class: "btn btn-create js-merge-button merge_when_build_succeeds" do Merge When Build Succeeds - = button_tag class: "btn btn-success dropdown-toggle", 'data-toggle' => 'dropdown' do - %span.caret - %span.sr-only - Select Merge Moment - %ul.js-merge-dropdown.dropdown-menu.dropdown-menu-right{ role: 'menu' } - %li - = link_to "#", class: "merge_when_build_succeeds" do - = icon('check fw') - Merge When Build Succeeds - %li - = link_to "#", class: "accept_merge_request" do - = icon('warning fw') - Merge Immediately + - unless @project.only_allow_merge_if_build_succeeds? + = button_tag class: "btn btn-success dropdown-toggle", 'data-toggle' => 'dropdown' do + %span.caret + %span.sr-only + Select Merge Moment + %ul.js-merge-dropdown.dropdown-menu.dropdown-menu-right{ role: 'menu' } + %li + = link_to "#", class: "merge_when_build_succeeds" do + = icon('check fw') + Merge When Build Succeeds + %li + = link_to "#", class: "accept_merge_request" do + = icon('warning fw') + Merge Immediately - else = f.button class: "btn btn-create btn-grouped js-merge-button accept_merge_request #{status_class}" do Accept Merge Request diff --git a/app/views/projects/merge_requests/widget/open/_build_failed.html.haml b/app/views/projects/merge_requests/widget/open/_build_failed.html.haml new file mode 100644 index 00000000000..14f51af5360 --- /dev/null +++ b/app/views/projects/merge_requests/widget/open/_build_failed.html.haml @@ -0,0 +1,6 @@ +%h4 + = icon('exclamation-triangle') + The build for this merge request failed + +%p + Please retry the build or push a new commit to fix the failure. diff --git a/app/views/shared/icons/_activity.svg b/app/views/shared/icons/_activity.svg index c87794b9062..d465504b154 100644 --- a/app/views/shared/icons/_activity.svg +++ b/app/views/shared/icons/_activity.svg @@ -3,13 +3,14 @@ <!-- Generator: Sketch 3.8.3 (29802) - http://www.bohemiancoding.com/sketch --> <title>path-1</title> <desc>Created with Sketch.</desc> - <defs> - <path d="M5,0 C4.448,0 4,0.448 4,1 L4,3 L1,3 C0.448,3 0,3.448 0,4 L0,9 C0,9.552 0.448,10 1,10 L5,10 L5,8 L11,8 L11,10 L15,10 C15.552,10 16,9.552 16,9 L16,4 C16,3.448 15.552,3 15,3 L12,3 L12,1 C12,0.448 11.552,0 11,0 L5,0 L5,0 L5,0 Z M6,2.5 C6,2.224 6.224,2 6.5,2 L9.5,2 C9.776,2 10,2.224 10,2.5 C10,2.776 9.776,3 9.5,3 L6.5,3 C6.224,3 6,2.776 6,2.5 L6,2.5 L6,2.5 Z M6,11 L10.001,11 L10.001,9 L6,9 L6,11 L6,11 L6,11 Z M11,11 L11,12 L5,12 L5,11 L1,11 C0.448,11 0,11.448 0,12 L0,15 C0,15.552 0.448,16 1,16 L15,16 C15.552,16 16,15.552 16,15 L16,12 C16,11.448 15.552,11 15,11 L11,11 L11,11 L11,11 Z" id="path-1"></path> - </defs> + <defs></defs> <g id="Page-1" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd"> - <mask id="mask-2" fill="white"> - <use xlink:href="#path-1"></use> - </mask> - <use id="path-1" fill="#D8D8D8" xlink:href="#path-1"></use> + <g id="_activity" fill="#7E7D7D"> + <g id="Page-1"> + <g id="path-1"> + <path d="M5,0 C4.448,0 4,0.448 4,1 L4,3 L1,3 C0.448,3 0,3.448 0,4 L0,9 C0,9.552 0.448,10 1,10 L5,10 L5,8 L11,8 L11,10 L15,10 C15.552,10 16,9.552 16,9 L16,4 C16,3.448 15.552,3 15,3 L12,3 L12,1 C12,0.448 11.552,0 11,0 L5,0 L5,0 L5,0 L5,0 Z M6,2.5 C6,2.224 6.224,2 6.5,2 L9.5,2 C9.776,2 10,2.224 10,2.5 C10,2.776 9.776,3 9.5,3 L6.5,3 C6.224,3 6,2.776 6,2.5 L6,2.5 L6,2.5 L6,2.5 Z M6,11 L10.001,11 L10.001,9 L6,9 L6,11 L6,11 L6,11 L6,11 Z M11,11 L11,12 L5,12 L5,11 L1,11 C0.448,11 0,11.448 0,12 L0,15 C0,15.552 0.448,16 1,16 L15,16 C15.552,16 16,15.552 16,15 L16,12 C16,11.448 15.552,11 15,11 L11,11 L11,11 L11,11 L11,11 Z"></path> + </g> + </g> + </g> </g> </svg>
\ No newline at end of file diff --git a/app/views/shared/issuable/_label_page_default.html.haml b/app/views/shared/issuable/_label_page_default.html.haml index 4e280c371ac..0acb8253139 100644 --- a/app/views/shared/issuable/_label_page_default.html.haml +++ b/app/views/shared/issuable/_label_page_default.html.haml @@ -4,7 +4,7 @@ - filter_placeholder = local_assigns.fetch(:filter_placeholder, 'Search labels') .dropdown-page-one = dropdown_title(title) - = dropdown_filter(filter_placeholder) + = dropdown_filter(filter_placeholder, search_id: "label-name") = dropdown_content - if @project && show_footer = dropdown_footer do diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 0510e7df597..1048ef6e243 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -181,7 +181,7 @@ production: &base # host: registry.example.com # port: 5005 # api_url: http://localhost:5000/ # internal address to the registry, will be used by GitLab to directly communicate with API - # key_path: config/registry.key + # key: config/registry.key # path: shared/registry # issuer: gitlab-issuer diff --git a/config/mail_room.yml b/config/mail_room.yml index 761a32adb9e..7cab24b295e 100644 --- a/config/mail_room.yml +++ b/config/mail_room.yml @@ -2,7 +2,7 @@ <% require "yaml" require "json" -require_relative "lib/gitlab/redis" +require_relative "lib/gitlab/redis" unless defined?(Gitlab::Redis) rails_env = ENV["RAILS_ENV"] || ENV["RACK_ENV"] || "development" diff --git a/db/migrate/20160419122101_add_only_allow_merge_if_build_succeeds_to_projects.rb b/db/migrate/20160419122101_add_only_allow_merge_if_build_succeeds_to_projects.rb new file mode 100644 index 00000000000..69d64ccd006 --- /dev/null +++ b/db/migrate/20160419122101_add_only_allow_merge_if_build_succeeds_to_projects.rb @@ -0,0 +1,15 @@ +class AddOnlyAllowMergeIfBuildSucceedsToProjects < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + def up + add_column_with_default(:projects, + :only_allow_merge_if_build_succeeds, + :boolean, + default: false) + end + + def down + remove_column(:projects, :only_allow_merge_if_build_succeeds) + end +end diff --git a/db/migrate/20160603075128_add_has_external_issue_tracker_to_projects.rb b/db/migrate/20160603075128_add_has_external_issue_tracker_to_projects.rb new file mode 100644 index 00000000000..be295f0181d --- /dev/null +++ b/db/migrate/20160603075128_add_has_external_issue_tracker_to_projects.rb @@ -0,0 +1,10 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddHasExternalIssueTrackerToProjects < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + def change + add_column(:projects, :has_external_issue_tracker, :boolean) + end +end diff --git a/db/schema.rb b/db/schema.rb index b7adf48fdb4..aac327797e7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -747,38 +747,40 @@ ActiveRecord::Schema.define(version: 20160608155312) do t.datetime "created_at" t.datetime "updated_at" t.integer "creator_id" - t.boolean "issues_enabled", default: true, null: false - t.boolean "merge_requests_enabled", default: true, null: false - t.boolean "wiki_enabled", default: true, null: false + t.boolean "issues_enabled", default: true, null: false + t.boolean "merge_requests_enabled", default: true, null: false + t.boolean "wiki_enabled", default: true, null: false t.integer "namespace_id" - t.string "issues_tracker", default: "gitlab", null: false + t.string "issues_tracker", default: "gitlab", null: false t.string "issues_tracker_id" - t.boolean "snippets_enabled", default: true, null: false + t.boolean "snippets_enabled", default: true, null: false t.datetime "last_activity_at" t.string "import_url" - t.integer "visibility_level", default: 0, null: false - t.boolean "archived", default: false, null: false + t.integer "visibility_level", default: 0, null: false + t.boolean "archived", default: false, null: false t.string "avatar" t.string "import_status" - t.float "repository_size", default: 0.0 - t.integer "star_count", default: 0, null: false + t.float "repository_size", default: 0.0 + t.integer "star_count", default: 0, null: false t.string "import_type" t.string "import_source" - t.integer "commit_count", default: 0 + t.integer "commit_count", default: 0 t.text "import_error" t.integer "ci_id" - t.boolean "builds_enabled", default: true, null: false - t.boolean "shared_runners_enabled", default: true, null: false + t.boolean "builds_enabled", default: true, null: false + t.boolean "shared_runners_enabled", default: true, null: false t.string "runners_token" t.string "build_coverage_regex" - t.boolean "build_allow_git_fetch", default: true, null: false - t.integer "build_timeout", default: 3600, null: false - t.boolean "pending_delete", default: false - t.boolean "public_builds", default: true, null: false - t.integer "pushes_since_gc", default: 0 + t.boolean "build_allow_git_fetch", default: true, null: false + t.integer "build_timeout", default: 3600, null: false + t.boolean "pending_delete", default: false + t.boolean "public_builds", default: true, null: false + t.integer "pushes_since_gc", default: 0 t.boolean "last_repository_check_failed" t.datetime "last_repository_check_at" t.boolean "container_registry_enabled" + t.boolean "only_allow_merge_if_build_succeeds", default: false, null: false + t.boolean "has_external_issue_tracker" end add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree diff --git a/doc/administration/container_registry.md b/doc/administration/container_registry.md index caf9a5bef2c..7870669fa77 100644 --- a/doc/administration/container_registry.md +++ b/doc/administration/container_registry.md @@ -62,7 +62,7 @@ registry: host: registry.gitlab.example.com port: 5005 api_url: http://localhost:5000/ - key_path: config/registry.key + key: config/registry.key path: shared/registry issuer: gitlab-issuer ``` @@ -75,7 +75,7 @@ where: | `host` | The host URL under which the Registry will run and the users will be able to use. | | `port` | The port under which the external Registry domain will listen on. | | `api_url` | The internal API URL under which the Registry is exposed to. It defaults to `http://localhost:5000`. | -| `key_path`| The private key location that is a pair of Registry's `rootcertbundle`. Read the [token auth configuration documentation][token-config]. | +| `key` | The private key location that is a pair of Registry's `rootcertbundle`. Read the [token auth configuration documentation][token-config]. | | `path` | This should be the same directory like specified in Registry's `rootdirectory`. Read the [storage configuration documentation][storage-config]. This path needs to be readable by the GitLab user, the web-server user and the Registry user. Read more in [#container-registry-storage-path](#container-registry-storage-path). | | `issuer` | This should be the same value as configured in Registry's `issuer`. Read the [token auth configuration documentation][token-config]. | diff --git a/doc/workflow/merge_requests.md b/doc/workflow/merge_requests.md index 1b5718c91c1..d2ec56e6504 100644 --- a/doc/workflow/merge_requests.md +++ b/doc/workflow/merge_requests.md @@ -2,6 +2,17 @@ Merge requests allow you to exchange changes you made to source code +## Only allow merge requests to be merged if the build succeeds + +You can prevent merge requests from being merged if their build did not succeed +in the project settings page. + + + +Navigate to project settings page and select the `Only allow merge requests to be merged if the build succeeds` check box. + +Please note that you need to have builds configured to enable this feature. + ## Checkout merge requests locally Locate the section for your GitLab remote in the `.git/config` file. It looks like this: diff --git a/doc/workflow/merge_requests/only_allow_merge_if_build_succeeds.png b/doc/workflow/merge_requests/only_allow_merge_if_build_succeeds.png Binary files differnew file mode 100644 index 00000000000..18bebf5fe92 --- /dev/null +++ b/doc/workflow/merge_requests/only_allow_merge_if_build_succeeds.png diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 43221d5622a..24df3e397e0 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -228,11 +228,10 @@ module API # Merge request can not be merged # because user dont have permissions to push into target branch unauthorized! unless merge_request.can_be_merged_by?(current_user) - not_allowed! if !merge_request.open? || merge_request.work_in_progress? - merge_request.check_if_can_be_merged + not_allowed! unless merge_request.mergeable_state? - render_api_error!('Branch cannot be merged', 406) unless merge_request.can_be_merged? + render_api_error!('Branch cannot be merged', 406) unless merge_request.mergeable? if params[:sha] && merge_request.source_sha != params[:sha] render_api_error!("SHA does not match HEAD of source branch: #{merge_request.source_sha}", 409) diff --git a/lib/ci/api/entities.rb b/lib/ci/api/entities.rb index b25e0e573a8..a902ced35d7 100644 --- a/lib/ci/api/entities.rb +++ b/lib/ci/api/entities.rb @@ -56,7 +56,7 @@ module Ci class TriggerRequest < Grape::Entity expose :id, :variables - expose :commit, using: Commit + expose :pipeline, using: Commit, as: :commit end end end diff --git a/spec/features/issues/filter_by_labels_spec.rb b/spec/features/issues/filter_by_labels_spec.rb index 0ec8b6b180a..16c619c9288 100644 --- a/spec/features/issues/filter_by_labels_spec.rb +++ b/spec/features/issues/filter_by_labels_spec.rb @@ -199,4 +199,19 @@ feature 'Issue filtering by Labels', feature: true do end end end + + context 'dropdown filtering', js: true do + it 'should filter by label name' do + page.within '.labels-filter' do + click_button 'Label' + wait_for_ajax + fill_in 'label-name', with: 'bug' + + page.within '.dropdown-content' do + expect(page).not_to have_content 'enhancement' + expect(page).to have_content 'bug' + end + end + end + end end diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index 7efbaaa048c..1f0594e6b02 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -294,40 +294,4 @@ describe 'Filter issues', feature: true do end end end - - describe 'filter by any author', js: true do - before do - user2 = create(:user, name: "tester") - create(:issue, project: project, author: user) - create(:issue, project: project, author: user2) - - visit namespace_project_issues_path(project.namespace, project) - end - - it 'should show filter by any author link' do - click_button "Author" - fill_in "Search authors", with: "tester" - - page.within ".dropdown-menu-author" do - expect(page).to have_content "tester" - end - end - - it 'should show filter issues by any author' do - page.within '.issues-list' do - expect(page).to have_selector ".issue", count: 2 - end - - click_button "Author" - fill_in "Search authors", with: "tester" - - page.within ".dropdown-menu-author" do - click_link "tester" - end - - page.within '.issues-list' do - expect(page).to have_selector ".issue", count: 1 - end - end - end end diff --git a/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb new file mode 100644 index 00000000000..65e9185ec24 --- /dev/null +++ b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb @@ -0,0 +1,105 @@ +require 'spec_helper' + +feature 'Only allow merge requests to be merged if the build succeeds', feature: true do + let(:project) { create(:project, :public) } + let(:merge_request) { create(:merge_request_with_diffs, source_project: project) } + + before do + login_as merge_request.author + + project.team << [merge_request.author, :master] + end + + context 'project does not have CI enabled' do + it 'allows MR to be merged' do + visit_merge_request(merge_request) + + expect(page).to have_button 'Accept Merge Request' + end + end + + context 'when project has CI enabled' do + let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) } + + context 'when merge requests can only be merged if the build succeeds' do + before do + project.update_attribute(:only_allow_merge_if_build_succeeds, true) + end + + context 'when CI is running' do + before { pipeline.update_column(:status, :running) } + + it 'does not allow to merge immediately' do + visit_merge_request(merge_request) + + expect(page).to have_button 'Merge When Build Succeeds' + expect(page).not_to have_button 'Select Merge Moment' + end + end + + context 'when CI failed' do + before { pipeline.update_column(:status, :failed) } + + it 'does not allow MR to be merged' do + visit_merge_request(merge_request) + + expect(page).not_to have_button 'Accept Merge Request' + expect(page).to have_content('Please retry the build or push a new commit to fix the failure.') + end + end + + context 'when CI succeeded' do + before { pipeline.update_column(:status, :success) } + + it 'allows MR to be merged' do + visit_merge_request(merge_request) + + expect(page).to have_button 'Accept Merge Request' + end + end + end + + context 'when merge requests can be merged when the build failed' do + before do + project.update_attribute(:only_allow_merge_if_build_succeeds, false) + end + + context 'when CI is running' do + before { pipeline.update_column(:status, :running) } + + it 'allows MR to be merged immediately', js: true do + visit_merge_request(merge_request) + + expect(page).to have_button 'Merge When Build Succeeds' + + click_button 'Select Merge Moment' + expect(page).to have_content 'Merge Immediately' + end + end + + context 'when CI failed' do + before { pipeline.update_column(:status, :failed) } + + it 'allows MR to be merged' do + visit_merge_request(merge_request) + + expect(page).to have_button 'Accept Merge Request' + end + end + + context 'when CI succeeded' do + before { pipeline.update_column(:status, :success) } + + it 'allows MR to be merged' do + visit_merge_request(merge_request) + + expect(page).to have_button 'Accept Merge Request' + end + end + end + end + + def visit_merge_request(merge_request) + visit namespace_project_merge_request_path(merge_request.project.namespace, merge_request.project, merge_request) + end +end diff --git a/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb b/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb index ea4ab2c852e..72bc6a0b704 100644 --- a/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/wiki_pipeline_spec.rb @@ -52,8 +52,8 @@ describe Banzai::Pipeline::WikiPipeline do end describe "Links" do - let(:namespace) { build_stubbed(:namespace, name: "wiki_link_ns") } - let(:project) { build_stubbed(:empty_project, :public, name: "wiki_link_project", namespace: namespace) } + let(:namespace) { create(:namespace, name: "wiki_link_ns") } + let(:project) { create(:empty_project, :public, name: "wiki_link_project", namespace: namespace) } let(:project_wiki) { ProjectWiki.new(project, double(:user)) } let(:page) { build(:wiki_page, wiki: project_wiki, page: OpenStruct.new(url_path: 'nested/twice/start-page')) } diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 7660ea2659c..2beb6cc598d 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -219,7 +219,7 @@ describe Ci::Build, models: true do context 'and trigger variables' do let(:trigger) { create(:ci_trigger, project: project) } - let(:trigger_request) { create(:ci_trigger_request_with_variables, commit: pipeline, trigger: trigger) } + let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) } let(:trigger_variables) do [ { key: :TRIGGER_KEY, value: 'TRIGGER_VALUE', public: false } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 1b7cbc3efda..3b199f4d98d 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -455,4 +455,157 @@ describe MergeRequest, models: true do expect(user2.assigned_open_merge_request_count).to eq(1) end end + + describe '#check_if_can_be_merged' do + let(:project) { create(:project, only_allow_merge_if_build_succeeds: true) } + + subject { create(:merge_request, source_project: project, merge_status: :unchecked) } + + context 'when it is not broken and has no conflicts' do + it 'is marked as mergeable' do + allow(subject).to receive(:broken?) { false } + allow(project).to receive_message_chain(:repository, :can_be_merged?) { true } + + expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged') + end + end + + context 'when broken' do + before { allow(subject).to receive(:broken?) { true } } + + it 'becomes unmergeable' do + expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged') + end + end + + context 'when it has conflicts' do + before do + allow(subject).to receive(:broken?) { false } + allow(project).to receive_message_chain(:repository, :can_be_merged?) { false } + end + + it 'becomes unmergeable' do + expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged') + end + end + end + + describe '#mergeable?' do + let(:project) { create(:project) } + + subject { create(:merge_request, source_project: project) } + + it 'returns false if #mergeable_state? is false' do + expect(subject).to receive(:mergeable_state?) { false } + + expect(subject.mergeable?).to be_falsey + end + + it 'return true if #mergeable_state? is true and the MR #can_be_merged? is true' do + allow(subject).to receive(:mergeable_state?) { true } + expect(subject).to receive(:check_if_can_be_merged) + expect(subject).to receive(:can_be_merged?) { true } + + expect(subject.mergeable?).to be_truthy + end + end + + describe '#mergeable_state?' do + let(:project) { create(:project) } + + subject { create(:merge_request, source_project: project) } + + it 'checks if merge request can be merged' do + allow(subject).to receive(:mergeable_ci_state?) { true } + expect(subject).to receive(:check_if_can_be_merged) + + subject.mergeable? + end + + context 'when not open' do + before { subject.close } + + it 'returns false' do + expect(subject.mergeable_state?).to be_falsey + end + end + + context 'when working in progress' do + before { subject.title = 'WIP MR' } + + it 'returns false' do + expect(subject.mergeable_state?).to be_falsey + end + end + + context 'when broken' do + before { allow(subject).to receive(:broken?) { true } } + + it 'returns false' do + expect(subject.mergeable_state?).to be_falsey + end + end + + context 'when failed' do + before { allow(subject).to receive(:broken?) { false } } + + context 'when project settings restrict to merge only if build succeeds and build failed' do + before do + project.only_allow_merge_if_build_succeeds = true + allow(subject).to receive(:mergeable_ci_state?) { false } + end + + it 'returns false' do + expect(subject.mergeable_state?).to be_falsey + end + end + end + end + + describe '#mergeable_ci_state?' do + let(:project) { create(:empty_project, only_allow_merge_if_build_succeeds: true) } + let(:pipeline) { create(:ci_empty_pipeline) } + + subject { build(:merge_request, target_project: project) } + + context 'when it is only allowed to merge when build is green' do + context 'and a failed pipeline is associated' do + before do + pipeline.statuses << create(:commit_status, status: 'failed', project: project) + allow(subject).to receive(:pipeline) { pipeline } + end + + it { expect(subject.mergeable_ci_state?).to be_falsey } + end + + context 'when no pipeline is associated' do + before do + allow(subject).to receive(:pipeline) { nil } + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end + end + + context 'when merges are not restricted to green builds' do + subject { build(:merge_request, target_project: build(:empty_project, only_allow_merge_if_build_succeeds: false)) } + + context 'and a failed pipeline is associated' do + before do + pipeline.statuses << create(:commit_status, status: 'failed', project: project) + allow(subject).to receive(:pipeline) { pipeline } + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end + + context 'when no pipeline is associated' do + before do + allow(subject).to receive(:pipeline) { nil } + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end + end + end end diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb index e771f35811e..ec81f05fc7a 100644 --- a/spec/models/project_services/bamboo_service_spec.rb +++ b/spec/models/project_services/bamboo_service_spec.rb @@ -194,7 +194,7 @@ describe BambooService, models: true do def service(bamboo_url: 'http://gitlab.com') described_class.create( - project: build_stubbed(:empty_project), + project: create(:empty_project), properties: { bamboo_url: bamboo_url, username: 'mic', diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb index ad24b895170..24a708ca849 100644 --- a/spec/models/project_services/teamcity_service_spec.rb +++ b/spec/models/project_services/teamcity_service_spec.rb @@ -182,7 +182,7 @@ describe TeamcityService, models: true do def service(teamcity_url: 'http://gitlab.com') described_class.create( - project: build_stubbed(:empty_project), + project: create(:empty_project), properties: { teamcity_url: teamcity_url, username: 'mic', diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 553556ed326..b6f36d3481e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -258,6 +258,69 @@ describe Project, models: true do end end + describe :external_issue_tracker do + let(:project) { create(:project) } + let(:ext_project) { create(:redmine_project) } + + context 'on existing projects with no value for has_external_issue_tracker' do + before(:each) do + project.update_column(:has_external_issue_tracker, nil) + ext_project.update_column(:has_external_issue_tracker, nil) + end + + it 'updates the has_external_issue_tracker boolean' do + expect do + project.external_issue_tracker + end.to change { project.reload.has_external_issue_tracker }.to(false) + + expect do + ext_project.external_issue_tracker + end.to change { ext_project.reload.has_external_issue_tracker }.to(true) + end + end + + it 'returns nil and does not query services when there is no external issue tracker' do + project.build_missing_services + project.reload + + expect(project).not_to receive(:services) + + expect(project.external_issue_tracker).to eq(nil) + end + + it 'retrieves external_issue_tracker querying services and cache it when there is external issue tracker' do + ext_project.reload # Factory returns a project with changed attributes + ext_project.build_missing_services + ext_project.reload + + expect(ext_project).to receive(:services).once.and_call_original + + 2.times { expect(ext_project.external_issue_tracker).to be_a_kind_of(RedmineService) } + end + end + + describe :cache_has_external_issue_tracker do + let(:project) { create(:project) } + + it 'stores true if there is any external_issue_tracker' do + services = double(:service, external_issue_trackers: [RedmineService.new]) + expect(project).to receive(:services).and_return(services) + + expect do + project.cache_has_external_issue_tracker + end.to change { project.has_external_issue_tracker}.to(true) + end + + it 'stores false if there is no external_issue_tracker' do + services = double(:service, external_issue_trackers: []) + expect(project).to receive(:services).and_return(services) + + expect do + project.cache_has_external_issue_tracker + end.to change { project.has_external_issue_tracker}.to(false) + end + end + describe :can_have_issues_tracker_id? do let(:project) { create(:project) } let(:ext_project) { create(:redmine_project) } diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 8592e112c50..2f000dbc01a 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -204,4 +204,37 @@ describe Service, models: true do expect(service.bamboo_url_was).to be_nil end end + + describe "callbacks" do + let(:project) { create(:project) } + let!(:service) do + RedmineService.new( + project: project, + active: true, + properties: { + project_url: 'http://redmine/projects/project_name_in_redmine', + issues_url: "http://redmine/#{project.id}/project_name_in_redmine/:id", + new_issue_url: 'http://redmine/projects/project_name_in_redmine/issues/new' + } + ) + end + + describe "on create" do + it "updates the has_external_issue_tracker boolean" do + expect do + service.save! + end.to change { service.project.has_external_issue_tracker }.from(nil).to(true) + end + end + + describe "on update" do + it "updates the has_external_issue_tracker boolean" do + service.save! + + expect do + service.update_attributes(active: false) + end.to change { service.project.has_external_issue_tracker }.from(true).to(false) + end + end + end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 9da69a913a8..1356f87b0e9 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -419,6 +419,15 @@ describe API::API, api: true do expect(json_response['message']).to eq('405 Method Not Allowed') end + it 'returns 405 if the build failed for a merge request that requires success' do + allow_any_instance_of(MergeRequest).to receive(:mergeable_ci_state?).and_return(false) + + put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) + + expect(response.status).to eq(405) + expect(json_response['message']).to eq('405 Method Not Allowed') + end + it "should return 401 if user has no permissions to merge" do user2 = create(:user) project.team << [user2, :reporter] diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 88271642532..e8508f8f950 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -85,7 +85,7 @@ describe Ci::API::API do trigger = FactoryGirl.create(:ci_trigger, project: project) pipeline = FactoryGirl.create(:ci_pipeline, project: project, ref: 'master') - trigger_request = FactoryGirl.create(:ci_trigger_request_with_variables, commit: pipeline, trigger: trigger) + trigger_request = FactoryGirl.create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) pipeline.create_builds(nil, trigger_request) project.variables << Ci::Variable.new(key: "SECRET_KEY", value: "secret_value") |
