diff options
68 files changed, 975 insertions, 166 deletions
@@ -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/users_select.js b/app/assets/javascripts/users_select.js index 3847278e80a..7a2221dbaf5 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -23,6 +23,8 @@ $dropdown = $(dropdown); options.projectId = $dropdown.data('project-id'); options.showCurrentUser = $dropdown.data('current-user'); + options.todoFilter = $dropdown.data('todo-filter'); + options.todoStateFilter = $dropdown.data('todo-state-filter'); showNullUser = $dropdown.data('null-user'); showMenuAbove = $dropdown.data('showMenuAbove'); showAnyUser = $dropdown.data('any-user'); @@ -394,6 +396,8 @@ project_id: options.projectId || null, group_id: options.groupId || null, skip_ldap: options.skipLdap || null, + todo_filter: options.todoFilter || null, + todo_state_filter: options.todoStateFilter || null, current_user: options.showCurrentUser || null, push_code_to_protected_branches: options.pushCodeToProtectedBranches || null, author_id: options.authorId || null, 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/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb index b48668eea87..daa82336208 100644 --- a/app/controllers/autocomplete_controller.rb +++ b/app/controllers/autocomplete_controller.rb @@ -11,9 +11,13 @@ class AutocompleteController < ApplicationController @users = @users.reorder(:name) @users = @users.page(params[:page]) + if params[:todo_filter].present? + @users = @users.todo_authors(current_user.id, params[:todo_state_filter]) + end + if params[:search].blank? # Include current user if available to filter by "Me" - if params[:current_user] && current_user + if params[:current_user].present? && current_user @users = [*@users, current_user] end 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/user.rb b/app/models/user.rb index af3c0b7dc02..65e96ee6b2e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -173,6 +173,7 @@ class User < ActiveRecord::Base scope :active, -> { with_state(:active) } scope :not_in_project, ->(project) { project.users.present? ? where("id not in (:ids)", ids: project.users.map(&:id) ) : all } scope :without_projects, -> { where('id NOT IN (SELECT DISTINCT(user_id) FROM members)') } + scope :todo_authors, ->(user_id, state) { where(id: Todo.where(user_id: user_id, state: state).select(:author_id)) } def self.with_two_factor joins("LEFT OUTER JOIN u2f_registrations AS u2f ON u2f.user_id = users.id"). 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/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml index 2411cc45724..e247eebc3fc 100644 --- a/app/views/dashboard/todos/index.html.haml +++ b/app/views/dashboard/todos/index.html.haml @@ -37,7 +37,7 @@ - if params[:author_id].present? = hidden_field_tag(:author_id, params[:author_id]) = dropdown_tag(user_dropdown_label(params[:author_id], 'Author'), options: { toggle_class: 'js-user-search js-filter-submit js-author-search', title: 'Filter by author', filter: true, filterInput: 'input#author-search', dropdown_class: 'dropdown-menu-user dropdown-menu-selectable dropdown-menu-author js-filter-submit', - placeholder: 'Search authors', data: { any_user: 'Any Author', first_user: (current_user.username if current_user), current_user: true, project_id: (@project.id if @project), selected: params[:author_id], field_name: 'author_id', default_label: 'Author' } }) + placeholder: 'Search authors', data: { any_user: 'Any Author', first_user: (current_user.username if current_user), project_id: (@project.id if @project), selected: params[:author_id], field_name: 'author_id', default_label: 'Author', todo_filter: true, todo_state_filter: params[:state] || 'pending' } }) .filter-item.inline - if params[:type].present? = hidden_field_tag(:type, params[:type]) 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/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/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/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/22588-todos-filter-shows-all-users.yml b/changelogs/unreleased/22588-todos-filter-shows-all-users.yml new file mode 100644 index 00000000000..1da72142880 --- /dev/null +++ b/changelogs/unreleased/22588-todos-filter-shows-all-users.yml @@ -0,0 +1,4 @@ +--- +title: 'Fix: Todos Filter Shows All Users' +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/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. + + + +From now on, you will not be able to merge from the UI until all discussions +are resolved. + + + [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.  -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. - + 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/project_hooks.rb b/lib/api/project_hooks.rb index dd93a85dc54..eef343c2ac6 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -1,114 +1,99 @@ module API # Projects API class ProjectHooks < Grape::API + helpers do + params :project_hook_properties do + requires :url, type: String, desc: "The URL to send the request to" + optional :push_events, type: Boolean, desc: "Trigger hook on push events" + optional :issues_events, type: Boolean, desc: "Trigger hook on issues events" + optional :merge_requests_events, type: Boolean, desc: "Trigger hook on merge request events" + optional :tag_push_events, type: Boolean, desc: "Trigger hook on tag push events" + optional :note_events, type: Boolean, desc: "Trigger hook on note(comment) events" + optional :build_events, type: Boolean, desc: "Trigger hook on build events" + optional :pipeline_events, type: Boolean, desc: "Trigger hook on pipeline events" + optional :wiki_events, type: Boolean, desc: "Trigger hook on wiki events" + optional :enable_ssl_verification, type: Boolean, desc: "Do SSL verification when triggering the hook" + optional :token, type: String, desc: "Secret token to validate received payloads; this will not be returned in the response" + end + end + before { authenticate! } before { authorize_admin_project } + params do + requires :id, type: String, desc: 'The ID of a project' + end resource :projects do - # Get project hooks - # - # Parameters: - # id (required) - The ID of a project - # Example Request: - # GET /projects/:id/hooks + desc 'Get project hooks' do + success Entities::ProjectHook + end get ":id/hooks" do - @hooks = paginate user_project.hooks - present @hooks, with: Entities::ProjectHook + hooks = paginate user_project.hooks + + present hooks, with: Entities::ProjectHook end - # Get a project hook - # - # Parameters: - # id (required) - The ID of a project - # hook_id (required) - The ID of a project hook - # Example Request: - # GET /projects/:id/hooks/:hook_id + desc 'Get a project hook' do + success Entities::ProjectHook + end + params do + requires :hook_id, type: Integer, desc: 'The ID of a project hook' + end get ":id/hooks/:hook_id" do - @hook = user_project.hooks.find(params[:hook_id]) - present @hook, with: Entities::ProjectHook + hook = user_project.hooks.find(params[:hook_id]) + present hook, with: Entities::ProjectHook end - # Add hook to project - # - # Parameters: - # id (required) - The ID of a project - # url (required) - The hook URL - # Example Request: - # POST /projects/:id/hooks + desc 'Add hook to project' do + success Entities::ProjectHook + end + params do + use :project_hook_properties + end post ":id/hooks" do - required_attributes! [:url] - attrs = attributes_for_keys [ - :url, - :push_events, - :issues_events, - :merge_requests_events, - :tag_push_events, - :note_events, - :build_events, - :pipeline_events, - :wiki_page_events, - :enable_ssl_verification, - :token - ] - @hook = user_project.hooks.new(attrs) + new_hook_params = declared(params, include_missing: false, include_parent_namespaces: false).to_h + hook = user_project.hooks.new(new_hook_params) - if @hook.save - present @hook, with: Entities::ProjectHook + if hook.save + present hook, with: Entities::ProjectHook else - if @hook.errors[:url].present? - error!("Invalid url given", 422) - end - not_found!("Project hook #{@hook.errors.messages}") + error!("Invalid url given", 422) if hook.errors[:url].present? + + not_found!("Project hook #{hook.errors.messages}") end end - # Update an existing project hook - # - # Parameters: - # id (required) - The ID of a project - # hook_id (required) - The ID of a project hook - # url (required) - The hook URL - # Example Request: - # PUT /projects/:id/hooks/:hook_id + desc 'Update an existing project hook' do + success Entities::ProjectHook + end + params do + requires :hook_id, type: Integer, desc: "The ID of the hook to update" + use :project_hook_properties + end put ":id/hooks/:hook_id" do - @hook = user_project.hooks.find(params[:hook_id]) - required_attributes! [:url] - attrs = attributes_for_keys [ - :url, - :push_events, - :issues_events, - :merge_requests_events, - :tag_push_events, - :note_events, - :build_events, - :pipeline_events, - :wiki_page_events, - :enable_ssl_verification, - :token - ] + hook = user_project.hooks.find(params[:hook_id]) + + new_params = declared(params, include_missing: false, include_parent_namespaces: false).to_h + new_params.delete('hook_id') - if @hook.update_attributes attrs - present @hook, with: Entities::ProjectHook + if hook.update_attributes(new_params) + present hook, with: Entities::ProjectHook else - if @hook.errors[:url].present? - error!("Invalid url given", 422) - end - not_found!("Project hook #{@hook.errors.messages}") + error!("Invalid url given", 422) if hook.errors[:url].present? + + not_found!("Project hook #{hook.errors.messages}") end end - # Deletes project hook. This is an idempotent function. - # - # Parameters: - # id (required) - The ID of a project - # hook_id (required) - The ID of hook to delete - # Example Request: - # DELETE /projects/:id/hooks/:hook_id + desc 'Deletes project hook' do + success Entities::ProjectHook + end + params do + requires :hook_id, type: Integer, desc: 'The ID of the hook to delete' + end delete ":id/hooks/:hook_id" do - required_attributes! [:hook_id] - begin - @hook = user_project.hooks.destroy(params[:hook_id]) + present user_project.hooks.destroy(params[:hook_id]), with: Entities::ProjectHook rescue # ProjectHook can raise Error if hook_id not found not_found!("Error deleting hook #{params[:hook_id]}") 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/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/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/features/todos/todos_filtering_spec.rb b/spec/features/todos/todos_filtering_spec.rb index b9e66243d84..d1f2bc78884 100644 --- a/spec/features/todos/todos_filtering_spec.rb +++ b/spec/features/todos/todos_filtering_spec.rb @@ -36,17 +36,54 @@ describe 'Dashboard > User filters todos', feature: true, js: true do expect(page).not_to have_content project_2.name_with_namespace end - it 'filters by author' do - click_button 'Author' - within '.dropdown-menu-author' do - fill_in 'Search authors', with: user_1.name - click_link user_1.name + context "Author filter" do + it 'filters by author' do + click_button 'Author' + + within '.dropdown-menu-author' do + fill_in 'Search authors', with: user_1.name + click_link user_1.name + end + + wait_for_ajax + + expect(find('.todos-list')).to have_content user_1.name + expect(find('.todos-list')).not_to have_content user_2.name end - wait_for_ajax + it "shows only authors of existing todos" do + click_button 'Author' + + within '.dropdown-menu-author' do + # It should contain two users + "Any Author" + expect(page).to have_selector('.dropdown-menu-user-link', count: 3) + expect(page).to have_content(user_1.name) + expect(page).to have_content(user_2.name) + end + end - expect(find('.todos-list')).to have_content user_1.name - expect(find('.todos-list')).not_to have_content user_2.name + it "shows only authors of existing done todos" do + user_3 = create :user + user_4 = create :user + create(:todo, user: user_1, author: user_3, project: project_1, target: issue, action: 1, state: :done) + create(:todo, user: user_1, author: user_4, project: project_2, target: merge_request, action: 2, state: :done) + + project_1.team << [user_3, :developer] + project_2.team << [user_4, :developer] + + visit dashboard_todos_path(state: 'done') + + click_button 'Author' + + within '.dropdown-menu-author' do + # It should contain two users + "Any Author" + expect(page).to have_selector('.dropdown-menu-user-link', count: 3) + expect(page).to have_content(user_3.name) + expect(page).to have_content(user_4.name) + expect(page).not_to have_content(user_1.name) + expect(page).not_to have_content(user_2.name) + end + end end it 'filters by type' do 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/user_spec.rb b/spec/models/user_spec.rb index d1ed774a914..ba47479a2e1 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -256,6 +256,20 @@ describe User, models: true do expect(users_without_two_factor).not_to include(user_with_2fa.id) end end + + describe '.todo_authors' do + it 'filters users' do + create :user + user_2 = create :user + user_3 = create :user + current_user = create :user + create(:todo, user: current_user, author: user_2, state: :done) + create(:todo, user: current_user, author: user_3, state: :pending) + + expect(User.todo_authors(current_user.id, 'pending')).to eq [user_3] + expect(User.todo_authors(current_user.id, 'done')).to eq [user_2] + end + end end describe "Respond to" 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 |
