diff options
77 files changed, 1020 insertions, 124 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 9411cc62003..7ed09fb1db8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ entry. - Show correct environment log in admin/logs (@duk3luk3 !7191) - Fix Milestone dropdown not stay selected for `Upcoming` and `No Milestone` option !7117 +- Diff collapse won't shift when collapsing. - Backups do not fail anymore when using tar on annex and custom_hooks only. !5814 - Adds user project membership expired event to clarify why user was removed (Callum Dryden) - Trim leading and trailing whitespace on project_path (Linus Thiel) @@ -13,6 +14,7 @@ entry. - Adds support for the `token` attribute in project hooks API (Gauvain Pocentek) - Adds an optional path parameter to the Commits API to filter commits by path (Luis HGO) - Fix Markdown styling inside reference links (Jan Zdráhal) +- Create new issue board list after creating a new label - Fix extra space on Build sidebar on Firefox !7060 - Fail gracefully when creating merge request with non-existing branch (alexsanford) - Fix mobile layout issues in admin user overview page !7087 @@ -66,6 +68,7 @@ entry. - In all filterable drop downs, put input field in focus only after load is complete (Ido @leibo) - Improve search query parameter naming in /admin/users !7115 (YarNayar) - Fix table pagination to be responsive +- Fix applying GitHub-imported labels when importing job is interrupted - Allow to search for user by secondary email address in the admin interface(/admin/users) !7115 (YarNayar) - Updated commit SHA styling on the branches page. diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 7ada0d303f3..3eefcb9dd5b 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -0.8.5 +1.0.0 @@ -100,7 +100,7 @@ gem 'seed-fu', '~> 2.3.5' # Markdown and HTML processing gem 'html-pipeline', '~> 1.11.0' -gem 'deckar01-task_list', '1.0.5', require: 'task_list/railtie' +gem 'deckar01-task_list', '1.0.6', require: 'task_list/railtie' gem 'gitlab-markup', '~> 1.5.0' gem 'redcarpet', '~> 3.3.3' gem 'RedCloth', '~> 4.3.2' diff --git a/Gemfile.lock b/Gemfile.lock index 3ecff5f6a68..6ea0578d9d2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -159,7 +159,7 @@ GEM database_cleaner (1.5.3) debug_inspector (0.0.2) debugger-ruby_core_source (1.3.8) - deckar01-task_list (1.0.5) + deckar01-task_list (1.0.6) activesupport (~> 4.0) html-pipeline rack (~> 1.0) @@ -840,7 +840,7 @@ DEPENDENCIES creole (~> 0.5.0) d3_rails (~> 3.5.0) database_cleaner (~> 1.5.0) - deckar01-task_list (= 1.0.5) + deckar01-task_list (= 1.0.6) default_value_for (~> 3.0.0) devise (~> 4.2) devise-two-factor (~> 3.0.0) diff --git a/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 b/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 index fe1a6dc7ea0..14f618fd5d5 100644 --- a/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 +++ b/app/assets/javascripts/boards/components/new_list_dropdown.js.es6 @@ -2,6 +2,19 @@ $(() => { const Store = gl.issueBoards.BoardsStore; + $(document).off('created.label').on('created.label', (e, label) => { + Store.new({ + title: label.title, + position: Store.state.lists.length - 2, + list_type: 'label', + label: { + id: label.id, + title: label.title, + color: label.color + } + }); + }); + $('.js-new-board-list').each(function () { const $this = $(this); new gl.CreateLabelDropdown($this.closest('.dropdown').find('.dropdown-new-label'), $this.data('namespace-path'), $this.data('project-path')); diff --git a/app/assets/javascripts/create_label.js.es6 b/app/assets/javascripts/create_label.js.es6 index f20580b1279..744aa0afa03 100644 --- a/app/assets/javascripts/create_label.js.es6 +++ b/app/assets/javascripts/create_label.js.es6 @@ -115,6 +115,8 @@ .show(); } else { this.$dropdownBack.trigger('click'); + + $(document).trigger('created.label', label); } }); } diff --git a/app/assets/javascripts/extensions/element.js.es6 b/app/assets/javascripts/extensions/element.js.es6 index c74fc9ad074..afb2f0d6956 100644 --- a/app/assets/javascripts/extensions/element.js.es6 +++ b/app/assets/javascripts/extensions/element.js.es6 @@ -1,5 +1,7 @@ -/* eslint-disable */ -Element.prototype.matches = Element.prototype.matches || Element.prototype.msMatches; +/* global Element */ +/* eslint-disable consistent-return, max-len */ + +Element.prototype.matches = Element.prototype.matches || Element.prototype.msMatchesSelector; Element.prototype.closest = function closest(selector, selectedElement = this) { if (!selectedElement) return; diff --git a/app/assets/stylesheets/pages/commit.scss b/app/assets/stylesheets/pages/commit.scss index 8ecf7fcb96d..47d3e72679b 100644 --- a/app/assets/stylesheets/pages/commit.scss +++ b/app/assets/stylesheets/pages/commit.scss @@ -36,9 +36,42 @@ padding: 10px 0; margin-bottom: 0; - .commit-options-dropdown-caret { - @media (max-width: $screen-sm) { - margin-left: 0; + @media (min-width: $screen-sm-min) { + display: flex; + align-items: center; + + .commit-meta { + flex: 1; + } + } + + .commit-hash-full { + @media (max-width: $screen-sm-max) { + width: 80px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + display: inline-block; + vertical-align: bottom; + } + } + + .commit-action-buttons { + i { + color: $gl-icon-color; + font-size: 13px; + margin-right: 3px; + } + + @media (max-width: $screen-xs-max) { + .dropdown { + width: 100%; + margin-top: 10px; + } + + .dropdown-toggle { + width: 100%; + } } } } @@ -188,17 +221,6 @@ } } -.commit-action-buttons { - position: relative; - top: -1px; - - i { - color: $gl-icon-color; - font-size: 13px; - margin-right: 3px; - } -} - /* * Commit message textarea for web editor and * custom merge request message diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 37600ed875c..517ad4f03f3 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -192,9 +192,10 @@ class ApplicationController < ActionController::Base end # JSON for infinite scroll via Pager object - def pager_json(partial, count) + def pager_json(partial, count, locals = {}) html = render_to_string( partial, + locals: locals, layout: false, formats: [:html] ) diff --git a/app/controllers/projects/commits_controller.rb b/app/controllers/projects/commits_controller.rb index c2e7bf1ffec..aba87b6144b 100644 --- a/app/controllers/projects/commits_controller.rb +++ b/app/controllers/projects/commits_controller.rb @@ -26,8 +26,15 @@ class Projects::CommitsController < Projects::ApplicationController respond_to do |format| format.html - format.json { pager_json("projects/commits/_commits", @commits.size) } format.atom { render layout: false } + + format.json do + pager_json( + 'projects/commits/_commits', + @commits.size, + project: @project, + ref: @ref) + end end end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 30f1cf4e5be..9f104d903cc 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -352,13 +352,23 @@ class Projects::MergeRequestsController < Projects::ApplicationController def branch_from # This is always source @source_project = @merge_request.nil? ? @project : @merge_request.source_project - @commit = @repository.commit(params[:ref]) if params[:ref].present? + + if params[:ref].present? + @ref = params[:ref] + @commit = @repository.commit(@ref) + end + render layout: false end def branch_to @target_project = selected_target_project - @commit = @target_project.commit(params[:ref]) if params[:ref].present? + + if params[:ref].present? + @ref = params[:ref] + @commit = @target_project.commit(@ref) + end + render layout: false end @@ -589,12 +599,27 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def merge_request_params - params.require(:merge_request).permit( - :title, :assignee_id, :source_project_id, :source_branch, - :target_project_id, :target_branch, :milestone_id, - :state_event, :description, :task_num, :force_remove_source_branch, - :lock_version, label_ids: [] - ) + params.require(:merge_request) + .permit(merge_request_params_ce) + end + + def merge_request_params_ce + [ + :assignee_id, + :description, + :force_remove_source_branch, + :lock_version, + :milestone_id, + :source_branch, + :source_project_id, + :state_event, + :target_branch, + :target_project_id, + :task_num, + :title, + + label_ids: [] + ] end def merge_params diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index bce5e29d8d8..6988527a3be 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -335,6 +335,7 @@ class ProjectsController < Projects::ApplicationController :visibility_level, :import_url, :last_activity_at, :namespace_id, :avatar, :build_allow_git_fetch, :build_timeout_in_minutes, :build_coverage_regex, :public_builds, :only_allow_merge_if_build_succeeds, :request_access_enabled, + :only_allow_merge_if_all_discussions_are_resolved, :lfs_enabled, project_feature_attributes ) end diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index fabe5c1f63a..895c3d728ad 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -56,10 +56,18 @@ module CiStatusHelper custom_icon(icon_name) end - def render_commit_status(commit, tooltip_placement: 'auto left') + def render_commit_status(commit, ref: nil, tooltip_placement: 'auto left') project = commit.project - path = pipelines_namespace_project_commit_path(project.namespace, project, commit) - render_status_with_link('commit', commit.status, path, tooltip_placement: tooltip_placement) + path = pipelines_namespace_project_commit_path( + project.namespace, + project, + commit) + + render_status_with_link( + 'commit', + commit.status(ref), + path, + tooltip_placement: tooltip_placement) end def render_pipeline_status(pipeline, tooltip_placement: 'auto left') diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 33dcee49aee..ed402b698fb 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -25,9 +25,11 @@ module CommitsHelper end end - def commit_to_html(commit, project, inline = true) - template = inline ? "inline_commit" : "commit" - render "projects/commits/#{template}", commit: commit, project: project unless commit.nil? + def commit_to_html(commit, ref, project) + render 'projects/commits/commit', + commit: commit, + ref: ref, + project: project end # Breadcrumb links for a Project and, if applicable, a tree path diff --git a/app/models/commit.rb b/app/models/commit.rb index e64fd1e0c1b..9e7fde9503d 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -226,12 +226,19 @@ class Commit end def pipelines - @pipeline ||= project.pipelines.where(sha: sha) + project.pipelines.where(sha: sha) end - def status - return @status if defined?(@status) - @status ||= pipelines.status + def status(ref = nil) + @statuses ||= {} + + if @statuses.key?(ref) + @statuses[ref] + elsif ref + @statuses[ref] = pipelines.where(ref: ref).status + else + @statuses[ref] = pipelines.status + end end def revert_branch_name diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 6b8ac3fb48b..d76feb9680e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -425,6 +425,7 @@ class MergeRequest < ActiveRecord::Base return false if work_in_progress? return false if broken? return false unless skip_ci_check || mergeable_ci_state? + return false unless mergeable_discussions_state? true end @@ -493,6 +494,12 @@ class MergeRequest < ActiveRecord::Base discussions_resolvable? && diff_discussions.none?(&:to_be_resolved?) end + def mergeable_discussions_state? + return true unless project.only_allow_merge_if_all_discussions_are_resolved? + + discussions_resolved? + end + def hook_attrs attrs = { source: source_project.try(:hook_attrs), diff --git a/app/models/project.rb b/app/models/project.rb index cf931f64c03..686d285410b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1067,10 +1067,6 @@ class Project < ActiveRecord::Base forks.count end - def find_label(name) - labels.find_by(name: name) - end - def origin_merge_requests merge_requests.where(source_project_id: self.id) end diff --git a/app/models/project_services/jira_service.rb b/app/models/project_services/jira_service.rb index 0a493b7a12b..2dbe0075465 100644 --- a/app/models/project_services/jira_service.rb +++ b/app/models/project_services/jira_service.rb @@ -163,6 +163,21 @@ class JiraService < IssueTrackerService add_comment(data, issue_key) end + # reason why service cannot be tested + def disabled_title + "Please fill in Password and Username." + end + + def can_test? + username.present? && password.present? + end + + # JIRA does not need test data. + # We are requesting the project that belongs to the project key. + def test_data(user = nil, project = nil) + nil + end + def test_settings return unless url.present? # Test settings by getting the project diff --git a/app/serializers/base_serializer.rb b/app/serializers/base_serializer.rb new file mode 100644 index 00000000000..de9a181db90 --- /dev/null +++ b/app/serializers/base_serializer.rb @@ -0,0 +1,18 @@ +class BaseSerializer + def initialize(parameters = {}) + @request = EntityRequest.new(parameters) + end + + def represent(resource, opts = {}) + self.class.entity_class + .represent(resource, opts.merge(request: @request)) + end + + def self.entity(entity_class) + @entity_class ||= entity_class + end + + def self.entity_class + @entity_class + end +end diff --git a/app/serializers/build_entity.rb b/app/serializers/build_entity.rb new file mode 100644 index 00000000000..3d9ac66de0e --- /dev/null +++ b/app/serializers/build_entity.rb @@ -0,0 +1,24 @@ +class BuildEntity < Grape::Entity + include RequestAwareEntity + + expose :id + expose :name + + expose :build_url do |build| + url_to(:namespace_project_build, build) + end + + expose :retry_url do |build| + url_to(:retry_namespace_project_build, build) + end + + expose :play_url, if: ->(build, _) { build.manual? } do |build| + url_to(:play_namespace_project_build, build) + end + + private + + def url_to(route, build) + send("#{route}_url", build.project.namespace, build.project, build) + end +end diff --git a/app/serializers/commit_entity.rb b/app/serializers/commit_entity.rb new file mode 100644 index 00000000000..f7eba6fc1e3 --- /dev/null +++ b/app/serializers/commit_entity.rb @@ -0,0 +1,12 @@ +class CommitEntity < API::Entities::RepoCommit + include RequestAwareEntity + + expose :author, using: UserEntity + + expose :commit_url do |commit| + namespace_project_tree_url( + request.project.namespace, + request.project, + id: commit.id) + end +end diff --git a/app/serializers/deployment_entity.rb b/app/serializers/deployment_entity.rb new file mode 100644 index 00000000000..ad6fc8d665b --- /dev/null +++ b/app/serializers/deployment_entity.rb @@ -0,0 +1,27 @@ +class DeploymentEntity < Grape::Entity + include RequestAwareEntity + + expose :id + expose :iid + expose :sha + + expose :ref do + expose :name do |deployment| + deployment.ref + end + + expose :ref_url do |deployment| + namespace_project_tree_url( + deployment.project.namespace, + deployment.project, + id: deployment.ref) + end + end + + expose :tag + expose :last? + expose :user, using: UserEntity + expose :commit, using: CommitEntity + expose :deployable, using: BuildEntity + expose :manual_actions, using: BuildEntity +end diff --git a/app/serializers/entity_request.rb b/app/serializers/entity_request.rb new file mode 100644 index 00000000000..456ba1174c0 --- /dev/null +++ b/app/serializers/entity_request.rb @@ -0,0 +1,12 @@ +class EntityRequest + # We use EntityRequest object to collect parameters and variables + # from the controller. Because options that are being passed to the entity + # do appear in each entity object in the chain, we need a way to pass data + # that is present in the controller (see #20045). + # + def initialize(parameters) + parameters.each do |key, value| + define_singleton_method(key) { value } + end + end +end diff --git a/app/serializers/environment_entity.rb b/app/serializers/environment_entity.rb new file mode 100644 index 00000000000..ee4392cc46d --- /dev/null +++ b/app/serializers/environment_entity.rb @@ -0,0 +1,20 @@ +class EnvironmentEntity < Grape::Entity + include RequestAwareEntity + + expose :id + expose :name + expose :state + expose :external_url + expose :environment_type + expose :last_deployment, using: DeploymentEntity + expose :stoppable? + + expose :environment_url do |environment| + namespace_project_environment_url( + environment.project.namespace, + environment.project, + environment) + end + + expose :created_at, :updated_at +end diff --git a/app/serializers/environment_serializer.rb b/app/serializers/environment_serializer.rb new file mode 100644 index 00000000000..91955542f25 --- /dev/null +++ b/app/serializers/environment_serializer.rb @@ -0,0 +1,3 @@ +class EnvironmentSerializer < BaseSerializer + entity EnvironmentEntity +end diff --git a/app/serializers/request_aware_entity.rb b/app/serializers/request_aware_entity.rb new file mode 100644 index 00000000000..ff8c1142abc --- /dev/null +++ b/app/serializers/request_aware_entity.rb @@ -0,0 +1,11 @@ +module RequestAwareEntity + extend ActiveSupport::Concern + + included do + include Gitlab::Routing.url_helpers + end + + def request + @options.fetch(:request) + end +end diff --git a/app/serializers/user_entity.rb b/app/serializers/user_entity.rb new file mode 100644 index 00000000000..43754ea94f7 --- /dev/null +++ b/app/serializers/user_entity.rb @@ -0,0 +1,2 @@ +class UserEntity < API::Entities::UserBasic +end diff --git a/app/views/projects/_last_commit.html.haml b/app/views/projects/_last_commit.html.haml index 630ae7d6140..8e23d51b224 100644 --- a/app/views/projects/_last_commit.html.haml +++ b/app/views/projects/_last_commit.html.haml @@ -1,7 +1,9 @@ -- if commit.status - = link_to builds_namespace_project_commit_path(commit.project.namespace, commit.project, commit), class: "ci-status ci-#{commit.status}" do - = ci_icon_for_status(commit.status) - = ci_label_for_status(commit.status) +- ref = local_assigns.fetch(:ref) +- status = commit.status(ref) +- if status + = link_to builds_namespace_project_commit_path(commit.project.namespace, commit.project, commit), class: "ci-status ci-#{status}" do + = ci_icon_for_status(status) + = ci_label_for_status(status) = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit_short_id" = link_to_gfm commit.title, namespace_project_commit_path(project.namespace, project, commit), class: "commit-row-message" diff --git a/app/views/projects/_merge_request_settings.html.haml b/app/views/projects/_merge_request_settings.html.haml index 80053dd501b..6e143c4b570 100644 --- a/app/views/projects/_merge_request_settings.html.haml +++ b/app/views/projects/_merge_request_settings.html.haml @@ -12,3 +12,7 @@ %span.descr Builds need to be configured to enable this feature. = link_to icon('question-circle'), help_page_path('user/project/merge_requests/merge_when_build_succeeds', anchor: 'only-allow-merge-requests-to-be-merged-if-the-build-succeeds') + .checkbox + = f.label :only_allow_merge_if_all_discussions_are_resolved do + = f.check_box :only_allow_merge_if_all_discussions_are_resolved + %strong Only allow merge requests to be merged if all discussions are resolved diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml index 3ffc3fcb7ac..149ee7c59d6 100644 --- a/app/views/projects/blob/_blob.html.haml +++ b/app/views/projects/blob/_blob.html.haml @@ -20,7 +20,7 @@ %ul.blob-commit-info.hidden-xs - blob_commit = @repository.last_commit_for_path(@commit.id, blob.path) - = render blob_commit, project: @project + = render blob_commit, project: @project, ref: @ref %div#blob-content-holder.blob-content-holder %article.file-holder diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index d8c95376b94..0ebc38d16cf 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -1,25 +1,25 @@ .commit-info-row.commit-info-row-header - %span.hidden-xs.hidden-sm Commit - = link_to @commit.short_id, namespace_project_commit_path(@project.namespace, @project, @commit), class: "monospace js-details-short" - = link_to("#", class: "js-details-expand hidden-xs hidden-sm") do - %span.text-expander - \... - %span.js-details-content.hide - = link_to @commit.id, namespace_project_commit_path(@project.namespace, @project, @commit), class: "monospace hidden-xs hidden-sm" - = clipboard_button(clipboard_text: @commit.id) - %span.hidden-xs authored - #{time_ago_with_tooltip(@commit.authored_date)} - %span by - = author_avatar(@commit, size: 24) - %strong - = commit_author_link(@commit, avatar: true, size: 24) - - if @commit.different_committer? - %span.light Committed by + .commit-meta + %strong Commit + %strong.monospace.js-details-short= @commit.short_id + = link_to("#", class: "js-details-expand hidden-xs hidden-sm") do + %span.text-expander + \... + %span.js-details-content.hide + %strong.monospace.commit-hash-full= @commit.id + = clipboard_button(clipboard_text: @commit.id) + %span.hidden-xs authored + #{time_ago_with_tooltip(@commit.authored_date)} + %span by + = author_avatar(@commit, size: 24) %strong - = commit_committer_link(@commit, avatar: true, size: 24) - #{time_ago_with_tooltip(@commit.committed_date)} - - .pull-right.commit-action-buttons + = commit_author_link(@commit, avatar: true, size: 24) + - if @commit.different_committer? + %span.light Committed by + %strong + = commit_committer_link(@commit, avatar: true, size: 24) + #{time_ago_with_tooltip(@commit.committed_date)} + .commit-action-buttons - if defined?(@notes_count) && @notes_count > 0 %span.btn.disabled.btn-grouped.hidden-xs.append-right-10 = icon('comment') @@ -28,8 +28,8 @@ Browse Files .dropdown.inline %a.btn.btn-default.dropdown-toggle{ data: { toggle: "dropdown" } } - %span.hidden-xs Options - = icon('caret-down', class: ".commit-options-dropdown-caret") + %span Options + = icon('caret-down') %ul.dropdown-menu.dropdown-menu-align-right %li.visible-xs-block.visible-sm-block = link_to namespace_project_tree_path(@project.namespace, @project, @commit) do diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index fb48aef0559..9f80a974d64 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -1,3 +1,4 @@ +- ref = local_assigns.fetch(:ref) - if @note_counts - note_count = @note_counts.fetch(commit.id, 0) - else @@ -18,15 +19,15 @@ %span.commit-row-message.visible-xs-inline · = commit.short_id - - if commit.status + - if commit.status(ref) .visible-xs-inline - = render_commit_status(commit) + = render_commit_status(commit, ref: ref) - if commit.description? %a.text-expander.hidden-xs.js-toggle-button ... .commit-actions.hidden-xs - - if commit.status - = render_commit_status(commit) + - if commit.status(ref) + = render_commit_status(commit, ref: ref) = clipboard_button(clipboard_text: commit.id) = link_to commit.short_id, namespace_project_commit_path(project.namespace, project, commit), class: "commit-short-id btn btn-transparent" = link_to_browse_code(project, commit) diff --git a/app/views/projects/commits/_commit_list.html.haml b/app/views/projects/commits/_commit_list.html.haml index 46e4de40042..ce416caa494 100644 --- a/app/views/projects/commits/_commit_list.html.haml +++ b/app/views/projects/commits/_commit_list.html.haml @@ -11,4 +11,4 @@ %li.warning-row.unstyled #{number_with_delimiter(hidden)} additional commits have been omitted to prevent performance issues. - else - %ul.content-list= render commits, project: @project + %ul.content-list= render commits, project: @project, ref: @ref diff --git a/app/views/projects/commits/_commits.html.haml b/app/views/projects/commits/_commits.html.haml index dd12eae8f7e..48756c68941 100644 --- a/app/views/projects/commits/_commits.html.haml +++ b/app/views/projects/commits/_commits.html.haml @@ -1,13 +1,11 @@ -- unless defined?(project) - - project = @project - +- ref = local_assigns.fetch(:ref) - commits, hidden = limited_commits(@commits) - commits.chunk { |c| c.committed_date.in_time_zone.to_date }.each do |day, commits| %li.commit-header= "#{day.strftime('%d %b, %Y')} #{pluralize(commits.count, 'commit')}" %li.commits-row %ul.list-unstyled.commit-list - = render commits, project: project + = render commits, project: project, ref: ref - if hidden > 0 %li.alert.alert-warning diff --git a/app/views/projects/commits/show.html.haml b/app/views/projects/commits/show.html.haml index 876c8002627..9628cbd1634 100644 --- a/app/views/projects/commits/show.html.haml +++ b/app/views/projects/commits/show.html.haml @@ -35,7 +35,7 @@ %div{id: dom_id(@project)} %ol#commits-list.list-unstyled.content_list - = render "commits", project: @project + = render 'commits', project: @project, ref: @ref = spinner :javascript diff --git a/app/views/projects/diffs/_file_header.html.haml b/app/views/projects/diffs/_file_header.html.haml index 73993f35b39..d3ed8e1bf38 100644 --- a/app/views/projects/diffs/_file_header.html.haml +++ b/app/views/projects/diffs/_file_header.html.haml @@ -1,4 +1,4 @@ -%i.fa.diff-toggle-caret +%i.fa.diff-toggle-caret.fa-fw - if defined?(blob) && blob && diff_file.submodule? %span = icon('archive fw') diff --git a/app/views/projects/merge_requests/branch_from.html.haml b/app/views/projects/merge_requests/branch_from.html.haml index 4f90dde6fa8..3837c4b388d 100644 --- a/app/views/projects/merge_requests/branch_from.html.haml +++ b/app/views/projects/merge_requests/branch_from.html.haml @@ -1 +1,2 @@ -= commit_to_html(@commit, @source_project, false) +- if @commit + = commit_to_html(@commit, @ref, @source_project) diff --git a/app/views/projects/merge_requests/branch_to.html.haml b/app/views/projects/merge_requests/branch_to.html.haml index 67a7a6bcec9..d69b71790a0 100644 --- a/app/views/projects/merge_requests/branch_to.html.haml +++ b/app/views/projects/merge_requests/branch_to.html.haml @@ -1 +1,2 @@ -= commit_to_html(@commit, @target_project, false) +- if @commit + = commit_to_html(@commit, @ref, @target_project) diff --git a/app/views/projects/merge_requests/show/_commits.html.haml b/app/views/projects/merge_requests/show/_commits.html.haml index 61020516bcf..a0e12fb3f38 100644 --- a/app/views/projects/merge_requests/show/_commits.html.haml +++ b/app/views/projects/merge_requests/show/_commits.html.haml @@ -3,4 +3,4 @@ Most recent commits displayed first %ol#commits-list.list-unstyled - = render "projects/commits/commits", project: @merge_request.source_project + = render "projects/commits/commits", project: @merge_request.source_project, ref: @merge_request.source_branch diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml index 842b6df310d..01314eb37d0 100644 --- a/app/views/projects/merge_requests/widget/_open.html.haml +++ b/app/views/projects/merge_requests/widget/_open.html.haml @@ -23,8 +23,10 @@ = 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? + - elsif !@merge_request.mergeable_ci_state? = render 'projects/merge_requests/widget/open/build_failed' + - elsif !@merge_request.mergeable_discussions_state? + = render 'projects/merge_requests/widget/open/unresolved_discussions' - elsif @merge_request.can_be_merged? || resolved_conflicts = render 'projects/merge_requests/widget/open/accept' diff --git a/app/views/projects/merge_requests/widget/open/_unresolved_discussions.html.haml b/app/views/projects/merge_requests/widget/open/_unresolved_discussions.html.haml new file mode 100644 index 00000000000..35d5677ee37 --- /dev/null +++ b/app/views/projects/merge_requests/widget/open/_unresolved_discussions.html.haml @@ -0,0 +1,6 @@ +%h4 + = icon('exclamation-triangle') + This merge request has unresolved discussions + +%p + Please resolve these discussions to allow this merge request to be merged.
\ No newline at end of file diff --git a/app/views/projects/services/_form.html.haml b/app/views/projects/services/_form.html.haml index 752fbc21a11..b41edeb2c7e 100644 --- a/app/views/projects/services/_form.html.haml +++ b/app/views/projects/services/_form.html.haml @@ -12,6 +12,9 @@ = form.submit 'Save changes', class: 'btn btn-save' - if @service.valid? && @service.activated? - - disabled = @service.can_test? ? '':'disabled' - = link_to 'Test settings', test_namespace_project_service_path(@project.namespace, @project, @service), class: "btn #{disabled}", title: @service.disabled_title + - unless @service.can_test? + - disabled_class = 'disabled' + - disabled_title = @service.disabled_title + + = link_to 'Test settings', test_namespace_project_service_path(@project.namespace, @project, @service), class: "btn #{disabled_class}", title: disabled_title = link_to "Cancel", namespace_project_services_path(@project.namespace, @project), class: "btn btn-cancel" diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml index d2570598501..4de95036eef 100644 --- a/app/views/projects/show.html.haml +++ b/app/views/projects/show.html.haml @@ -79,7 +79,7 @@ = render 'shared/notifications/button', notification_setting: @notification_setting - if @repository.commit .project-last-commit{ class: container_class } - = render 'projects/last_commit', commit: @repository.commit, project: @project + = render 'projects/last_commit', commit: @repository.commit, ref: current_ref, project: @project %div{ class: container_class } - if @project.archived? diff --git a/changelogs/unreleased/20968-add-setting-to-check-unresolved-discussion.yml b/changelogs/unreleased/20968-add-setting-to-check-unresolved-discussion.yml new file mode 100644 index 00000000000..8f03746ff80 --- /dev/null +++ b/changelogs/unreleased/20968-add-setting-to-check-unresolved-discussion.yml @@ -0,0 +1,4 @@ +--- +title: Add setting to only allow merge requests to be merged when all discussions are resolved +merge_request: 7125 +author: Rodolfo Arruda diff --git a/changelogs/unreleased/issue_23032.yml b/changelogs/unreleased/issue_23032.yml new file mode 100644 index 00000000000..d376cf52112 --- /dev/null +++ b/changelogs/unreleased/issue_23032.yml @@ -0,0 +1,4 @@ +--- +title: Allow to test JIRA service settings without having a repository +merge_request: +author: diff --git a/changelogs/unreleased/show-status-from-branch.yml b/changelogs/unreleased/show-status-from-branch.yml new file mode 100644 index 00000000000..1afc230c05c --- /dev/null +++ b/changelogs/unreleased/show-status-from-branch.yml @@ -0,0 +1,4 @@ +--- +title: Fix showing pipeline status for a given commit from correct branch +merge_request: 7034 +author: diff --git a/db/migrate/20160914131004_only_allow_merge_if_all_discussions_are_resolved.rb b/db/migrate/20160914131004_only_allow_merge_if_all_discussions_are_resolved.rb new file mode 100644 index 00000000000..fad62d716b3 --- /dev/null +++ b/db/migrate/20160914131004_only_allow_merge_if_all_discussions_are_resolved.rb @@ -0,0 +1,17 @@ +class OnlyAllowMergeIfAllDiscussionsAreResolved < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + disable_ddl_transaction! + + def up + add_column_with_default(:projects, + :only_allow_merge_if_all_discussions_are_resolved, + :boolean, + default: false) + end + + def down + remove_column(:projects, :only_allow_merge_if_all_discussions_are_resolved) + end +end diff --git a/db/schema.rb b/db/schema.rb index dc088925d97..5476b0c93e5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -905,6 +905,7 @@ ActiveRecord::Schema.define(version: 20161103171205) do t.boolean "has_external_wiki" t.boolean "lfs_enabled" t.text "description_html" + t.boolean "only_allow_merge_if_all_discussions_are_resolved", default: false, null: false end add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree diff --git a/doc/api/projects.md b/doc/api/projects.md index 4f4b20a1874..bbb3bfb4995 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -89,6 +89,7 @@ Parameters: "public_builds": true, "shared_with_groups": [], "only_allow_merge_if_build_succeeds": false, + "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false }, { @@ -151,6 +152,7 @@ Parameters: "public_builds": true, "shared_with_groups": [], "only_allow_merge_if_build_succeeds": false, + "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false } ] @@ -429,6 +431,7 @@ Parameters: } ], "only_allow_merge_if_build_succeeds": false, + "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false } ``` @@ -602,6 +605,7 @@ Parameters: | `import_url` | string | no | URL to import repository from | | `public_builds` | boolean | no | If `true`, builds can be viewed by non-project-members | | `only_allow_merge_if_build_succeeds` | boolean | no | Set whether merge requests can only be merged with successful builds | +| `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | @@ -634,6 +638,7 @@ Parameters: | `import_url` | string | no | URL to import repository from | | `public_builds` | boolean | no | If `true`, builds can be viewed by non-project-members | | `only_allow_merge_if_build_succeeds` | boolean | no | Set whether merge requests can only be merged with successful builds | +| `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | @@ -665,6 +670,7 @@ Parameters: | `import_url` | string | no | URL to import repository from | | `public_builds` | boolean | no | If `true`, builds can be viewed by non-project-members | | `only_allow_merge_if_build_succeeds` | boolean | no | Set whether merge requests can only be merged with successful builds | +| `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `lfs_enabled` | boolean | no | Enable LFS | | `request_access_enabled` | boolean | no | Allow users to request member access | @@ -752,6 +758,7 @@ Example response: "public_builds": true, "shared_with_groups": [], "only_allow_merge_if_build_succeeds": false, + "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false } ``` @@ -820,6 +827,7 @@ Example response: "public_builds": true, "shared_with_groups": [], "only_allow_merge_if_build_succeeds": false, + "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false } ``` @@ -908,6 +916,7 @@ Example response: "public_builds": true, "shared_with_groups": [], "only_allow_merge_if_build_succeeds": false, + "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false } ``` @@ -996,6 +1005,7 @@ Example response: "public_builds": true, "shared_with_groups": [], "only_allow_merge_if_build_succeeds": false, + "only_allow_merge_if_all_discussions_are_resolved": false, "request_access_enabled": false } ``` diff --git a/doc/install/installation.md b/doc/install/installation.md index 7e947e4b2ba..b5e2640b380 100644 --- a/doc/install/installation.md +++ b/doc/install/installation.md @@ -403,7 +403,7 @@ If you are not using Linux you may have to run `gmake` instead of cd /home/git sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-workhorse.git cd gitlab-workhorse - sudo -u git -H git checkout v0.8.5 + sudo -u git -H git checkout v1.0.0 sudo -u git -H make ### Initialize Database and Activate Advanced Features diff --git a/doc/update/8.13-to-8.14.md b/doc/update/8.13-to-8.14.md index 787511fd6cf..46ea19d11d0 100644 --- a/doc/update/8.13-to-8.14.md +++ b/doc/update/8.13-to-8.14.md @@ -84,7 +84,7 @@ GitLab 8.1. ```bash cd /home/git/gitlab-workhorse sudo -u git -H git fetch --all -sudo -u git -H git checkout v0.8.5 +sudo -u git -H git checkout v1.0.0 sudo -u git -H make ``` diff --git a/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved.png b/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved.png Binary files differnew file mode 100644 index 00000000000..52c8acf15e0 --- /dev/null +++ b/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved.png diff --git a/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved_msg.png b/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved_msg.png Binary files differnew file mode 100644 index 00000000000..79ba5c362c7 --- /dev/null +++ b/doc/user/project/merge_requests/img/only_allow_merge_if_all_discussions_are_resolved_msg.png diff --git a/doc/user/project/merge_requests/merge_request_discussion_resolution.md b/doc/user/project/merge_requests/merge_request_discussion_resolution.md index 2559f5f5250..285b1798ac5 100644 --- a/doc/user/project/merge_requests/merge_request_discussion_resolution.md +++ b/doc/user/project/merge_requests/merge_request_discussion_resolution.md @@ -33,7 +33,25 @@ resolved discussions tracker. !["3/4 discussions resolved"][discussions-resolved] +## Only allow merge requests to be merged if all discussions are resolved + +> [Introduced][ce-7125] in GitLab 8.14. + +You can prevent merge requests from being merged until all discussions are resolved. + +Navigate to your project's settings page, select the +**Only allow merge requests to be merged if all discussions are resolved** check +box and hit **Save** for the changes to take effect. + +![Only allow merge if all the discussions are resolved settings](img/only_allow_merge_if_all_discussions_are_resolved.png) + +From now on, you will not be able to merge from the UI until all discussions +are resolved. + +![Only allow merge if all the discussions are resolved message](img/only_allow_merge_if_all_discussions_are_resolved_msg.png) + [ce-5022]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022 +[ce-7125]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7125 [resolve-discussion-button]: img/resolve_discussion_button.png [resolve-comment-button]: img/resolve_comment_button.png [discussion-view]: img/discussion_view.png diff --git a/doc/user/project/merge_requests/merge_when_build_succeeds.md b/doc/user/project/merge_requests/merge_when_build_succeeds.md index c138061fd40..d4e5b5de685 100644 --- a/doc/user/project/merge_requests/merge_when_build_succeeds.md +++ b/doc/user/project/merge_requests/merge_when_build_succeeds.md @@ -40,7 +40,7 @@ hit **Save** for the changes to take effect. ![Only allow merge if build succeeds settings](img/merge_when_build_succeeds_only_if_succeeds_settings.png) -From now on, every time the pipelinefails you will not be able to merge the +From now on, every time the pipeline fails you will not be able to merge the merge request from the UI, until you make all relevant builds pass. -![Only allow merge if build succeeds msg](img/merge_when_build_succeeds_only_if_succeeds_msg.png) +![Only allow merge if build succeeds message](img/merge_when_build_succeeds_only_if_succeeds_msg.png) diff --git a/features/steps/shared/diff_note.rb b/features/steps/shared/diff_note.rb index 4df4e89f5b9..35b71599708 100644 --- a/features/steps/shared/diff_note.rb +++ b/features/steps/shared/diff_note.rb @@ -210,7 +210,7 @@ module SharedDiffNote end step 'I click side-by-side diff button' do - find('#parallel-diff-btn').click + find('#parallel-diff-btn').trigger('click') end step 'I see side-by-side diff button' do diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 1f378ba1635..01e31f6f7d1 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -100,6 +100,7 @@ module API end expose :only_allow_merge_if_build_succeeds expose :request_access_enabled + expose :only_allow_merge_if_all_discussions_are_resolved end class Member < UserBasic diff --git a/lib/api/labels.rb b/lib/api/labels.rb index 326e1e7ae00..238cea00fba 100644 --- a/lib/api/labels.rb +++ b/lib/api/labels.rb @@ -25,7 +25,7 @@ module API post ':id/labels' do authorize! :admin_label, user_project - label = user_project.find_label(params[:name]) + label = available_labels.find_by(title: params[:name]) conflict!('Label already exists') if label label = user_project.labels.create(declared(params, include_parent_namespaces: false).to_h) @@ -46,7 +46,7 @@ module API delete ':id/labels' do authorize! :admin_label, user_project - label = user_project.find_label(params[:name]) + label = user_project.labels.find_by(title: params[:name]) not_found!('Label') unless label present label.destroy, with: Entities::Label, current_user: current_user @@ -65,7 +65,7 @@ module API put ':id/labels' do authorize! :admin_label, user_project - label = user_project.find_label(params[:name]) + label = user_project.labels.find_by(title: params[:name]) not_found!('Label not found') unless label update_params = declared(params, diff --git a/lib/api/projects.rb b/lib/api/projects.rb index da16e24d7ea..6b856128c2e 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -139,7 +139,8 @@ module API :shared_runners_enabled, :snippets_enabled, :visibility_level, - :wiki_enabled] + :wiki_enabled, + :only_allow_merge_if_all_discussions_are_resolved] attrs = map_public_to_visibility_level(attrs) @project = ::Projects::CreateService.new(current_user, attrs).execute if @project.saved? @@ -193,7 +194,8 @@ module API :shared_runners_enabled, :snippets_enabled, :visibility_level, - :wiki_enabled] + :wiki_enabled, + :only_allow_merge_if_all_discussions_are_resolved] attrs = map_public_to_visibility_level(attrs) @project = ::Projects::CreateService.new(user, attrs).execute if @project.saved? @@ -275,7 +277,8 @@ module API :shared_runners_enabled, :snippets_enabled, :visibility_level, - :wiki_enabled] + :wiki_enabled, + :only_allow_merge_if_all_discussions_are_resolved] attrs = map_public_to_visibility_level(attrs) authorize_admin_project authorize! :rename_project, user_project if attrs[:name].present? diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index ecc28799737..90cf38a8513 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -52,13 +52,14 @@ module Gitlab fetch_resources(:labels, repo, per_page: 100) do |labels| labels.each do |raw| begin - label = LabelFormatter.new(project, raw).create! - @labels[label.title] = label.id + LabelFormatter.new(project, raw).create! rescue => e errors << { type: :label, url: Gitlab::UrlSanitizer.sanitize(raw.url), errors: e.message } end end end + + cache_labels! end def import_milestones @@ -234,6 +235,12 @@ module Gitlab end end + def cache_labels! + project.labels.select(:id, :title).find_each do |label| + @labels[label.title] = label.id + end + end + def fetch_resources(resource_type, *opts) return if imported?(resource_type) diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 940d54f8686..49127aecc63 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -297,6 +297,72 @@ describe Projects::MergeRequestsController do end end end + + describe 'only_allow_merge_if_all_discussions_are_resolved? setting' do + let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project, author: user) } + + context 'when enabled' do + before do + project.update_column(:only_allow_merge_if_all_discussions_are_resolved, true) + end + + context 'with unresolved discussion' do + before do + expect(merge_request).not_to be_discussions_resolved + end + + it 'returns :failed' do + merge_with_sha + + expect(assigns(:status)).to eq(:failed) + end + end + + context 'with all discussions resolved' do + before do + merge_request.discussions.each { |d| d.resolve!(user) } + expect(merge_request).to be_discussions_resolved + end + + it 'returns :success' do + merge_with_sha + + expect(assigns(:status)).to eq(:success) + end + end + end + + context 'when disabled' do + before do + project.update_column(:only_allow_merge_if_all_discussions_are_resolved, false) + end + + context 'with unresolved discussion' do + before do + expect(merge_request).not_to be_discussions_resolved + end + + it 'returns :success' do + merge_with_sha + + expect(assigns(:status)).to eq(:success) + end + end + + context 'with all discussions resolved' do + before do + merge_request.discussions.each { |d| d.resolve!(user) } + expect(merge_request).to be_discussions_resolved + end + + it 'returns :success' do + merge_with_sha + + expect(assigns(:status)).to eq(:success) + end + end + end + end end end diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb index f780e01253c..37eb49c94df 100644 --- a/spec/factories/merge_requests.rb +++ b/spec/factories/merge_requests.rb @@ -68,6 +68,11 @@ FactoryGirl.define do factory :closed_merge_request, traits: [:closed] factory :reopened_merge_request, traits: [:reopened] factory :merge_request_with_diffs, traits: [:with_diffs] + factory :merge_request_with_diff_notes do + after(:create) do |mr| + create(:diff_note_on_merge_request, noteable: mr, project: mr.source_project) + end + end factory :labeled_merge_request do transient do diff --git a/spec/features/boards/boards_spec.rb b/spec/features/boards/boards_spec.rb index a92075fec8f..6cb8753e8fc 100644 --- a/spec/features/boards/boards_spec.rb +++ b/spec/features/boards/boards_spec.rb @@ -380,6 +380,25 @@ describe 'Issue Boards', feature: true, js: true do wait_for_board_cards(1, 5) end + + it 'creates new list from a new label' do + click_button 'Create new list' + + wait_for_ajax + + click_link 'Create new label' + + fill_in('new_label_name', with: 'Testing New Label') + + first('.suggest-colors a').click + + click_button 'Create' + + wait_for_ajax + wait_for_vue_resource + + expect(page).to have_selector('.board', count: 5) + end end end diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb index 338c53f08a6..44646ffc602 100644 --- a/spec/features/commits_spec.rb +++ b/spec/features/commits_spec.rb @@ -12,11 +12,15 @@ describe 'Commits' do end let!(:pipeline) do - FactoryGirl.create :ci_pipeline, project: project, sha: project.commit.sha + create(:ci_pipeline, + project: project, + ref: project.default_branch, + sha: project.commit.sha, + status: :success) end context 'commit status is Generic Commit Status' do - let!(:status) { FactoryGirl.create :generic_commit_status, pipeline: pipeline } + let!(:status) { create(:generic_commit_status, pipeline: pipeline) } before do project.team << [@user, :reporter] @@ -39,7 +43,7 @@ describe 'Commits' do end context 'commit status is Ci Build' do - let!(:build) { FactoryGirl.create :ci_build, pipeline: pipeline } + let!(:build) { create(:ci_build, pipeline: pipeline) } let(:artifacts_file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') } context 'when logged as developer' do @@ -48,13 +52,22 @@ describe 'Commits' do end describe 'Project commits' do + let!(:pipeline_from_other_branch) do + create(:ci_pipeline, + project: project, + ref: 'fix', + sha: project.commit.sha, + status: :failed) + end + before do visit namespace_project_commits_path(project.namespace, project, :master) end - it 'shows build status' do + it 'shows correct build status from default branch' do page.within("//li[@id='commit-#{pipeline.short_sha}']") do - expect(page).to have_css(".ci-status-link") + expect(page).to have_css('.ci-status-link') + expect(page).to have_css('.ci-status-icon-success') end end end diff --git a/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions.rb b/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions.rb new file mode 100644 index 00000000000..7f11db3c417 --- /dev/null +++ b/spec/features/merge_requests/check_if_mergeable_with_unresolved_discussions.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +feature 'Check if mergeable with unresolved discussions', js: true, feature: true do + let(:user) { create(:user) } + let(:project) { create(:project) } + let!(:merge_request) { create(:merge_request_with_diff_notes, source_project: project, author: user) } + + before do + login_as user + project.team << [user, :master] + end + + context 'when project.only_allow_merge_if_all_discussions_are_resolved == true' do + before do + project.update_column(:only_allow_merge_if_all_discussions_are_resolved, true) + end + + context 'with unresolved discussions' do + it 'does not allow to merge' do + visit_merge_request(merge_request) + + expect(page).not_to have_button 'Accept Merge Request' + expect(page).to have_content('This merge request has unresolved discussions') + end + end + + context 'with all discussions resolved' do + before do + merge_request.discussions.each { |d| d.resolve!(user) } + end + + 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 project.only_allow_merge_if_all_discussions_are_resolved == false' do + before do + project.update_column(:only_allow_merge_if_all_discussions_are_resolved, false) + end + + context 'with unresolved discussions' do + it 'does not allow to merge' do + visit_merge_request(merge_request) + + expect(page).to have_button 'Accept Merge Request' + end + end + + context 'with all discussions resolved' do + before do + merge_request.discussions.each { |d| d.resolve!(user) } + end + + it 'allows MR to be merged' do + visit_merge_request(merge_request) + + expect(page).to have_button 'Accept Merge Request' + 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/models/commit_spec.rb b/spec/models/commit_spec.rb index 51be3f36135..e3bb3482d67 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -205,12 +205,53 @@ eos end end - describe '#ci_commits' do - # TODO: kamil - end - describe '#status' do - # TODO: kamil + context 'without arguments for compound status' do + shared_examples 'giving the status from pipeline' do + it do + expect(commit.status).to eq(Ci::Pipeline.status) + end + end + + context 'with pipelines' do + let!(:pipeline) do + create(:ci_empty_pipeline, project: project, sha: commit.sha) + end + + it_behaves_like 'giving the status from pipeline' + end + + context 'without pipelines' do + it_behaves_like 'giving the status from pipeline' + end + end + + context 'when a particular ref is specified' do + let!(:pipeline_from_master) do + create(:ci_empty_pipeline, + project: project, + sha: commit.sha, + ref: 'master', + status: 'failed') + end + + let!(:pipeline_from_fix) do + create(:ci_empty_pipeline, + project: project, + sha: commit.sha, + ref: 'fix', + status: 'success') + end + + it 'gives pipelines from a particular branch' do + expect(commit.status('master')).to eq(pipeline_from_master.status) + expect(commit.status('fix')).to eq(pipeline_from_fix.status) + end + + it 'gives compound status if ref is nil' do + expect(commit.status(nil)).to eq(commit.status) + end + end end describe '#participants' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 1067ff7bb4d..fb032a89d50 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -825,11 +825,8 @@ describe MergeRequest, models: true do 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 + context 'when #mergeable_ci_state? is false' do before do - project.only_allow_merge_if_build_succeeds = true allow(subject).to receive(:mergeable_ci_state?) { false } end @@ -837,6 +834,16 @@ describe MergeRequest, models: true do expect(subject.mergeable_state?).to be_falsey end end + + context 'when #mergeable_discussions_state? is false' do + before do + allow(subject).to receive(:mergeable_discussions_state?) { false } + end + + it 'returns false' do + expect(subject.mergeable_state?).to be_falsey + end + end end end @@ -887,7 +894,49 @@ describe MergeRequest, models: true do end end - describe '#environments' do + describe '#mergeable_discussions_state?' do + let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) } + + context 'when project.only_allow_merge_if_all_discussions_are_resolved == true' do + let(:project) { create(:project, only_allow_merge_if_all_discussions_are_resolved: true) } + + context 'with all discussions resolved' do + before do + merge_request.discussions.each { |d| d.resolve!(merge_request.author) } + end + + it 'returns true' do + expect(merge_request.mergeable_discussions_state?).to be_truthy + end + end + + context 'with unresolved discussions' do + before do + merge_request.discussions.each(&:unresolve!) + end + + it 'returns false' do + expect(merge_request.mergeable_discussions_state?).to be_falsey + end + end + end + + context 'when project.only_allow_merge_if_all_discussions_are_resolved == false' do + let(:project) { create(:project, only_allow_merge_if_all_discussions_are_resolved: false) } + + context 'with unresolved discussions' do + before do + merge_request.discussions.each(&:unresolve!) + end + + it 'returns true' do + expect(merge_request.mergeable_discussions_state?).to be_truthy + end + end + end + end + + describe "#environments" do let(:project) { create(:project) } let(:merge_request) { create(:merge_request, source_project: project) } diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index ee0e38bd373..05ee4a08391 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -33,6 +33,41 @@ describe JiraService, models: true do end end + describe '#can_test?' do + let(:jira_service) { described_class.new } + + it 'returns false if username is blank' do + allow(jira_service).to receive_messages( + url: 'http://jira.example.com', + username: '', + password: '12345678' + ) + + expect(jira_service.can_test?).to be_falsy + end + + it 'returns false if password is blank' do + allow(jira_service).to receive_messages( + url: 'http://jira.example.com', + username: 'tester', + password: '' + ) + + expect(jira_service.can_test?).to be_falsy + end + + it 'returns true if password and username are present' do + jira_service = described_class.new + allow(jira_service).to receive_messages( + url: 'http://jira.example.com', + username: 'tester', + password: '12345678' + ) + + expect(jira_service.can_test?).to be_truthy + end + end + describe "Execute" do let(:user) { create(:user) } let(:project) { create(:project) } @@ -46,16 +81,19 @@ describe JiraService, models: true do service_hook: true, url: 'http://jira.example.com', username: 'gitlab_jira_username', - password: 'gitlab_jira_password' + password: 'gitlab_jira_password', + project_key: 'GitLabProject' ) @jira_service.save - project_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123' - @transitions_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions' - @comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment' + project_issues_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123' + @project_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/project/GitLabProject' + @transitions_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/transitions' + @comment_url = 'http://gitlab_jira_username:gitlab_jira_password@jira.example.com/rest/api/2/issue/JIRA-123/comment' - WebMock.stub_request(:get, project_url) + WebMock.stub_request(:get, @project_url) + WebMock.stub_request(:get, project_issues_url) WebMock.stub_request(:post, @transitions_url) WebMock.stub_request(:post, @comment_url) end @@ -99,6 +137,14 @@ describe JiraService, models: true do body: /this-is-a-custom-id/ ).once end + + context "when testing" do + it "tries to get jira project" do + @jira_service.execute(nil) + + expect(WebMock).to have_requested(:get, @project_url) + end + end end describe "Stored password invalidation" do diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 46641fcd846..f702dfaaf53 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -82,7 +82,20 @@ describe API::API, api: true do expect(json_response['message']['title']).to eq(['is invalid']) end - it 'returns 409 if label already exists' do + it 'returns 409 if label already exists in group' do + group = create(:group) + group_label = create(:group_label, group: group) + project.update(group: group) + + post api("/projects/#{project.id}/labels", user), + name: group_label.name, + color: '#FFAABB' + + expect(response).to have_http_status(409) + expect(json_response['message']).to eq('Label already exists') + end + + it 'returns 409 if label already exists in project' do post api("/projects/#{project.id}/labels", user), name: 'label1', color: '#FFAABB' diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 973928d007a..3c8f0ac531a 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -256,7 +256,8 @@ describe API::API, api: true do merge_requests_enabled: false, wiki_enabled: false, only_allow_merge_if_build_succeeds: false, - request_access_enabled: true + request_access_enabled: true, + only_allow_merge_if_all_discussions_are_resolved: false }) post api('/projects', user), project @@ -327,6 +328,22 @@ describe API::API, api: true do expect(json_response['only_allow_merge_if_build_succeeds']).to be_truthy end + it 'sets a project as allowing merge even if discussions are unresolved' do + project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: false }) + + post api('/projects', user), project + + expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_falsey + end + + it 'sets a project as allowing merge only if all discussions are resolved' do + project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: true }) + + post api('/projects', user), project + + expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_truthy + end + context 'when a visibility level is restricted' do before do @project = attributes_for(:project, { public: true }) @@ -448,6 +465,22 @@ describe API::API, api: true do post api("/projects/user/#{user.id}", admin), project expect(json_response['only_allow_merge_if_build_succeeds']).to be_truthy end + + it 'sets a project as allowing merge even if discussions are unresolved' do + project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: false }) + + post api("/projects/user/#{user.id}", admin), project + + expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_falsey + end + + it 'sets a project as allowing merge only if all discussions are resolved' do + project = attributes_for(:project, { only_allow_merge_if_all_discussions_are_resolved: true }) + + post api("/projects/user/#{user.id}", admin), project + + expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_truthy + end end describe "POST /projects/:id/uploads" do @@ -509,6 +542,7 @@ describe API::API, api: true do expect(json_response['shared_with_groups'][0]['group_name']).to eq(group.name) expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access) expect(json_response['only_allow_merge_if_build_succeeds']).to eq(project.only_allow_merge_if_build_succeeds) + expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) end it 'returns a project by path name' do diff --git a/spec/serializers/build_entity_spec.rb b/spec/serializers/build_entity_spec.rb new file mode 100644 index 00000000000..2734f5bedca --- /dev/null +++ b/spec/serializers/build_entity_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe BuildEntity do + let(:entity) do + described_class.new(build, request: double) + end + + subject { entity.as_json } + + context 'when build is a regular job' do + let(:build) { create(:ci_build) } + + it 'contains url to build page and retry action' do + expect(subject).to include(:build_url, :retry_url) + expect(subject).not_to include(:play_url) + end + + it 'does not contain sensitive information' do + expect(subject).not_to include(/token/) + expect(subject).not_to include(/variables/) + end + end + + context 'when build is a manual action' do + let(:build) { create(:ci_build, :manual) } + + it 'contains url to play action' do + expect(subject).to include(:play_url) + end + end +end diff --git a/spec/serializers/commit_entity_spec.rb b/spec/serializers/commit_entity_spec.rb new file mode 100644 index 00000000000..628e35c9a28 --- /dev/null +++ b/spec/serializers/commit_entity_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe CommitEntity do + let(:entity) do + described_class.new(commit, request: request) + end + + let(:request) { double('request') } + let(:project) { create(:project) } + let(:commit) { project.commit } + + subject { entity.as_json } + + before do + allow(request).to receive(:project).and_return(project) + end + + context 'when commit author is a user' do + before do + create(:user, email: commit.author_email) + end + + it 'contains information about user' do + expect(subject.fetch(:author)).not_to be_nil + end + end + + context 'when commit author is not a user' do + it 'does not contain author details' do + expect(subject.fetch(:author)).to be_nil + end + end + + it 'contains commit URL' do + expect(subject).to include(:commit_url) + end + + it 'needs to receive project in the request' do + expect(request).to receive(:project) + .and_return(project) + + subject + end +end diff --git a/spec/serializers/deployment_entity_spec.rb b/spec/serializers/deployment_entity_spec.rb new file mode 100644 index 00000000000..51b6de91571 --- /dev/null +++ b/spec/serializers/deployment_entity_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe DeploymentEntity do + let(:entity) do + described_class.new(deployment, request: double) + end + + let(:deployment) { create(:deployment) } + + subject { entity.as_json } + + it 'exposes internal deployment id' do + expect(subject).to include(:iid) + end + + it 'exposes nested information about branch' do + expect(subject[:ref][:name]).to eq 'master' + expect(subject[:ref][:ref_url]).not_to be_empty + end +end diff --git a/spec/serializers/entity_request_spec.rb b/spec/serializers/entity_request_spec.rb new file mode 100644 index 00000000000..86654adfd54 --- /dev/null +++ b/spec/serializers/entity_request_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe EntityRequest do + subject do + described_class.new(user: 'user', project: 'some project') + end + + describe 'methods created' do + it 'defines accessible attributes' do + expect(subject.user).to eq 'user' + expect(subject.project).to eq 'some project' + end + + it 'raises error when attribute is not defined' do + expect { subject.some_method }.to raise_error NoMethodError + end + end +end diff --git a/spec/serializers/environment_entity_spec.rb b/spec/serializers/environment_entity_spec.rb new file mode 100644 index 00000000000..4ca8c299147 --- /dev/null +++ b/spec/serializers/environment_entity_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe EnvironmentEntity do + let(:entity) do + described_class.new(environment, request: double) + end + + let(:environment) { create(:environment) } + subject { entity.as_json } + + it 'exposes latest deployment' do + expect(subject).to include(:last_deployment) + end + + it 'exposes core elements of environment' do + expect(subject).to include(:id, :name, :state, :environment_url) + end +end diff --git a/spec/serializers/environment_serializer_spec.rb b/spec/serializers/environment_serializer_spec.rb new file mode 100644 index 00000000000..37bc086826c --- /dev/null +++ b/spec/serializers/environment_serializer_spec.rb @@ -0,0 +1,60 @@ +require 'spec_helper' + +describe EnvironmentSerializer do + let(:serializer) do + described_class + .new(user: user, project: project) + .represent(resource) + end + + let(:json) { serializer.as_json } + let(:user) { create(:user) } + let(:project) { create(:project) } + + context 'when there is a single object provided' do + before do + create(:ci_build, :manual, name: 'manual1', + pipeline: deployable.pipeline) + end + + let(:deployment) do + create(:deployment, deployable: deployable, + user: user, + project: project, + sha: project.commit.id) + end + + let(:deployable) { create(:ci_build) } + let(:resource) { deployment.environment } + + it 'it generates payload for single object' do + expect(json).to be_an_instance_of Hash + end + + it 'contains important elements of environment' do + expect(json) + .to include(:name, :external_url, :environment_url, :last_deployment) + end + + it 'contains relevant information about last deployment' do + last_deployment = json.fetch(:last_deployment) + + expect(last_deployment) + .to include(:ref, :user, :commit, :deployable, :manual_actions) + end + end + + context 'when there is a collection of objects provided' do + let(:project) { create(:empty_project) } + let(:resource) { create_list(:environment, 2) } + + it 'contains important elements of environment' do + expect(json.first) + .to include(:last_deployment, :name, :external_url) + end + + it 'generates payload for collection' do + expect(json).to be_an_instance_of Array + end + end +end diff --git a/spec/serializers/user_entity_spec.rb b/spec/serializers/user_entity_spec.rb new file mode 100644 index 00000000000..c5d11cbcf5e --- /dev/null +++ b/spec/serializers/user_entity_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe UserEntity do + let(:entity) { described_class.new(user) } + let(:user) { create(:user) } + subject { entity.as_json } + + it 'exposes user name and login' do + expect(subject).to include(:username, :name) + end + + it 'does not expose passwords' do + expect(subject).not_to include(/password/) + end + + it 'does not expose tokens' do + expect(subject).not_to include(/token/) + end + + it 'does not expose 2FA OTPs' do + expect(subject).not_to include(/otp/) + end +end |