diff options
56 files changed, 679 insertions, 149 deletions
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 043c6a11c4f..e43afbb4cc9 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -89,6 +89,14 @@ // Set the default path for all cookies to GitLab's root directory Cookies.defaults.path = gon.relative_url_root || '/'; + // `hashchange` is not triggered when link target is already in window.location + $body.on('click', 'a[href^="#"]', function() { + var href = this.getAttribute('href'); + if (href.substr(1) === gl.utils.getLocationHash()) { + setTimeout(gl.utils.handleLocationHash, 1); + } + }); + // prevent default action for disabled buttons $('.btn').click(function(e) { if ($(this).hasClass('disabled')) { diff --git a/app/assets/javascripts/profile/profile.js.es6 b/app/assets/javascripts/profile/profile.js.es6 index eb2fe3477a2..aef2e3a3fa8 100644 --- a/app/assets/javascripts/profile/profile.js.es6 +++ b/app/assets/javascripts/profile/profile.js.es6 @@ -41,15 +41,12 @@ } beforeUpdateUsername() { - $('.loading-username').show(); - $(this).find('.update-success').hide(); - return $(this).find('.update-failed').hide(); + $('.loading-username', this).removeClass('hidden'); } afterUpdateUsername() { - $('.loading-username').hide(); - $(this).find('.btn-save').enable(); - return $(this).find('.loading-gif').hide(); + $('.loading-username', this).addClass('hidden'); + $('button[type=submit]', this).enable(); } onUpdateNotifs(e, data) { diff --git a/app/assets/stylesheets/framework/gitlab-theme.scss b/app/assets/stylesheets/framework/gitlab-theme.scss index 1b52a6a8e71..d6566dc4ec9 100644 --- a/app/assets/stylesheets/framework/gitlab-theme.scss +++ b/app/assets/stylesheets/framework/gitlab-theme.scss @@ -80,6 +80,9 @@ } } + .about-gitlab { + color: $color-light; + } } } } diff --git a/app/assets/stylesheets/framework/lists.scss b/app/assets/stylesheets/framework/lists.scss index b34f3bf6abc..277d4202950 100644 --- a/app/assets/stylesheets/framework/lists.scss +++ b/app/assets/stylesheets/framework/lists.scss @@ -199,6 +199,7 @@ ul.content-list { display: -webkit-flex; display: -ms-flexbox; display: flex; + align-items: center; white-space: nowrap; } diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index 4f84bc37f90..5abda13a963 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -116,8 +116,8 @@ padding-top: 16px; padding-bottom: 11px; display: inline-block; - width: 50%; line-height: 28px; + white-space: normal; /* Small devices (phones, tablets, 768px and lower) */ @media (max-width: $screen-xs-max) { @@ -158,7 +158,6 @@ } .nav-controls { - width: 50%; display: inline-block; float: right; text-align: right; @@ -167,7 +166,9 @@ > .btn, > .btn-container, - > .dropdown { + > .dropdown, + > input, + > form { margin-right: $gl-padding-top; display: inline-block; vertical-align: top; @@ -182,19 +183,21 @@ float: none; } - > form { - display: inline-block; - } - .icon-label { display: none; } - input { + .btn, + .dropdown, + .dropdown-toggle, + input, + form { height: 35px; + } + + input { display: inline-block; position: relative; - margin-right: $gl-padding-top; /* Medium devices (desktops, 992px and up) */ @media (min-width: $screen-md-min) { width: 200px; } @@ -218,6 +221,7 @@ .btn, form, .dropdown, + .dropdown-toggle, .dropdown-menu-toggle, .form-control { margin: 0 0 10px; @@ -256,6 +260,10 @@ .nav-text, .nav-controls { width: auto; + + @media (max-width: $screen-xs-max) { + width: 100%; + } } } @@ -268,6 +276,10 @@ padding: 17px 0; } } + + pre { + width: 100%; + } } .layout-nav { @@ -423,33 +435,38 @@ } @media (max-width: $screen-xs-max) { - .top-area .nav-controls { - $controls-margin: $btn-xs-side-margin - 2px; + .top-area { + flex-flow: row wrap; - &.controls-flex { - display: flex; - flex-flow: row wrap; - align-items: center; - justify-content: center; - padding: 0 0 $gl-padding-top; - } + .nav-controls { + $controls-margin: $btn-xs-side-margin - 2px; + flex: 0 0 100%; + + &.controls-flex { + display: flex; + flex-flow: row wrap; + align-items: center; + justify-content: center; + padding: 0 0 $gl-padding-top; + } - .controls-item, - .controls-item:last-child { - flex: 1 1 35%; - display: block; - width: 100%; - margin: $controls-margin; + .controls-item, + .controls-item-full, + .controls-item:last-child { + flex: 1 1 35%; + display: block; + width: 100%; + margin: $controls-margin; - .btn, - .dropdown { - margin: 0; + .btn, + .dropdown { + margin: 0; + } } - } - .controls-item-full { - @extend .controls-item; - flex: 1 1 100%; + .controls-item-full { + flex: 1 1 100%; + } } } } diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index 46a06cd7eab..a8641e83154 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -101,6 +101,17 @@ padding: 0 8px; border-radius: 6px; } + + .about-gitlab { + padding: 7px $gl-sidebar-padding; + font-size: $gl-font-size; + line-height: 24px; + display: block; + text-decoration: none; + font-weight: normal; + position: absolute; + bottom: 10px; + } } .sidebar-action-buttons { diff --git a/app/assets/stylesheets/pages/login.scss b/app/assets/stylesheets/pages/login.scss index dd27a06fcd2..712bd3da22f 100644 --- a/app/assets/stylesheets/pages/login.scss +++ b/app/assets/stylesheets/pages/login.scss @@ -105,19 +105,19 @@ li { flex: 1; text-align: center; + border-left: 1px solid $border-color; &:first-of-type { + border-left: none; border-top-left-radius: $border-radius-default; } &:last-of-type { - border-left: 1px solid $border-color; border-top-right-radius: $border-radius-default; } &:not(.active) { background-color: $gray-light; - border-left: 1px solid $border-color; } a { diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 01c8fa2739f..b61f4e9a2db 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -42,6 +42,8 @@ class GroupsController < Groups::ApplicationController @notification_setting = current_user.notification_settings_for(group) end + @nested_groups = group.children + setup_projects respond_to do |format| diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb index 953091492ae..e2d9d5ed460 100644 --- a/app/controllers/projects/tags_controller.rb +++ b/app/controllers/projects/tags_controller.rb @@ -8,7 +8,7 @@ class Projects::TagsController < Projects::ApplicationController before_action :authorize_admin_project!, only: [:destroy] def index - params[:sort] = params[:sort].presence || 'name' + params[:sort] = params[:sort].presence || sort_value_recently_updated @sort = params[:sort] @tags = TagsFinder.new(@repository, params).execute diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index feeb553e607..c6c63918fa5 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -173,48 +173,27 @@ module ProjectsHelper nav_tabs << :merge_requests end - if can?(current_user, :read_pipeline, project) - nav_tabs << :pipelines - end - - if can?(current_user, :read_build, project) - nav_tabs << :builds - end - if Gitlab.config.registry.enabled && can?(current_user, :read_container_image, project) nav_tabs << :container_registry end - if can?(current_user, :read_environment, project) - nav_tabs << :environments - end - - if can?(current_user, :admin_project, project) - nav_tabs << :settings - end - - if can?(current_user, :read_project_member, project) - nav_tabs << :team - end - - if can?(current_user, :read_issue, project) - nav_tabs << :issues - end - - if can?(current_user, :read_wiki, project) - nav_tabs << :wiki - end - - if can?(current_user, :read_project_snippet, project) - nav_tabs << :snippets - end - - if can?(current_user, :read_label, project) - nav_tabs << :labels - end + tab_ability_map = { + environments: :read_environment, + milestones: :read_milestone, + pipelines: :read_pipeline, + snippets: :read_project_snippet, + settings: :admin_project, + builds: :read_build, + labels: :read_label, + issues: :read_issue, + team: :read_project_member, + wiki: :read_wiki + } - if can?(current_user, :read_milestone, project) - nav_tabs << :milestones + tab_ability_map.each do |tab, ability| + if can?(current_user, ability, project) + nav_tabs << tab + end end nav_tabs.flatten diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 0ea7b1b1098..5e63825bf99 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -92,8 +92,9 @@ module Issuable after_save :record_metrics def update_assignee_cache_counts - # make sure we flush the cache for both the old *and* new assignee - User.find(assignee_id_was).update_cache_counts if assignee_id_was + # make sure we flush the cache for both the old *and* new assignees(if they exist) + previous_assignee = User.find_by_id(assignee_id_was) if assignee_id_was + previous_assignee.update_cache_counts if previous_assignee assignee.update_cache_counts if assignee end diff --git a/app/models/group.rb b/app/models/group.rb index 85696ad9747..9888b242e98 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -161,15 +161,17 @@ class Group < Namespace end def has_owner?(user) - owners.include?(user) + members_with_parents.owners.where(user_id: user).any? end def has_master?(user) - members.masters.where(user_id: user).any? + members_with_parents.masters.where(user_id: user).any? end + # Check if user is a last owner of the group. + # Parent owners are ignored for nested groups. def last_owner?(user) - has_owner?(user) && owners.size == 1 + owners.include?(user) && owners.size == 1 end def avatar_type @@ -195,6 +197,14 @@ class Group < Namespace end def refresh_members_authorized_projects - UserProjectAccessChangedService.new(users.pluck(:id)).execute + UserProjectAccessChangedService.new(users_with_parents.pluck(:id)).execute + end + + def members_with_parents + GroupMember.where(requested_at: nil, source_id: parents.map(&:id).push(id)) + end + + def users_with_parents + User.where(id: members_with_parents.select(:user_id)) end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 22490d121c7..61845bf4036 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -576,11 +576,7 @@ class MergeRequest < ActiveRecord::Base ext = Gitlab::ReferenceExtractor.new(project, current_user) ext.analyze(description) - issues = ext.issues - closing_issues = Gitlab::ClosingIssueExtractor.new(project, current_user). - closed_by_message(description) - - issues - closing_issues + ext.issues - closes_issues end def target_project_path diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 6f943feb2a7..0be6e113655 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -4,7 +4,7 @@ class GroupPolicy < BasePolicy return unless @user globally_viewable = @subject.public? || (@subject.internal? && !@user.external?) - member = @subject.users.include?(@user) + member = @subject.users_with_parents.include?(@user) owner = @user.admin? || @subject.has_owner?(@user) master = owner || @subject.has_master?(@user) diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index b5db9c12622..71ef8901932 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -171,9 +171,7 @@ class ProjectPolicy < BasePolicy def disabled_features! repository_enabled = project.feature_available?(:repository, user) - unless project.feature_available?(:issues, user) - cannot!(*named_abilities(:issue)) - end + block_issues_abilities unless project.feature_available?(:merge_requests, user) && repository_enabled cannot!(*named_abilities(:merge_request)) @@ -245,11 +243,20 @@ class ProjectPolicy < BasePolicy def project_group_member?(user) project.group && ( - project.group.members.exists?(user_id: user.id) || + project.group.members_with_parents.exists?(user_id: user.id) || project.group.requesters.exists?(user_id: user.id) ) end + def block_issues_abilities + unless project.feature_available?(:issues, user) + cannot! :read_issue if project.default_issues_tracker? + cannot! :create_issue + cannot! :update_issue + cannot! :admin_issue + end + end + def named_abilities(name) [ :"read_#{name}", diff --git a/app/services/groups/create_service.rb b/app/services/groups/create_service.rb index 2bccd584dde..febeb661fb5 100644 --- a/app/services/groups/create_service.rb +++ b/app/services/groups/create_service.rb @@ -12,6 +12,13 @@ module Groups return @group end + if @group.parent && !can?(current_user, :admin_group, @group.parent) + @group.parent = nil + @group.errors.add(:parent_id, 'manage access required to create subgroup') + + return @group + end + @group.name ||= @group.path.dup @group.save @group.add_owner(current_user) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index ab3d2a9a0cd..4ce5fd993d9 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -36,14 +36,10 @@ class IssuableBaseService < BaseService end end - def filter_params(issuable_ability_name = :issue) - filter_assignee - filter_milestone - filter_labels + def filter_params(issuable) + ability_name = :"admin_#{issuable.to_ability_name}" - ability = :"admin_#{issuable_ability_name}" - - unless can?(current_user, ability, project) + unless can?(current_user, ability_name, project) params.delete(:milestone_id) params.delete(:labels) params.delete(:add_label_ids) @@ -52,14 +48,35 @@ class IssuableBaseService < BaseService params.delete(:assignee_id) params.delete(:due_date) end + + filter_assignee(issuable) + filter_milestone + filter_labels end - def filter_assignee - if params[:assignee_id] == IssuableFinder::NONE - params[:assignee_id] = '' + def filter_assignee(issuable) + return unless params[:assignee_id].present? + + assignee_id = params[:assignee_id] + + if assignee_id.to_s == IssuableFinder::NONE + params[:assignee_id] = "" + else + params.delete(:assignee_id) unless assignee_can_read?(issuable, assignee_id) end end + def assignee_can_read?(issuable, assignee_id) + new_assignee = User.find_by_id(assignee_id) + + return false unless new_assignee.present? + + ability_name = :"read_#{issuable.to_ability_name}" + resource = issuable.persisted? ? issuable : project + + can?(new_assignee, ability_name, resource) + end + def filter_milestone milestone_id = params[:milestone_id] return unless milestone_id @@ -138,7 +155,7 @@ class IssuableBaseService < BaseService def create(issuable) merge_slash_commands_into_params!(issuable) - filter_params + filter_params(issuable) params.delete(:state_event) params[:author] ||= current_user @@ -180,7 +197,7 @@ class IssuableBaseService < BaseService change_state(issuable) change_subscription(issuable) change_todo(issuable) - filter_params + filter_params(issuable) old_labels = issuable.labels.to_a old_mentioned_users = issuable.mentioned_users.to_a diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 742e834df97..35af867a098 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -17,10 +17,6 @@ module Issues private - def filter_params - super(:issue) - end - def execute_hooks(issue, action = 'open') issue_data = hook_data(issue, action) hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 800fd39c424..70e25956dc7 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -38,10 +38,6 @@ module MergeRequests private - def filter_params - super(:merge_request) - end - def merge_requests_for(branch) origin_merge_requests = @project.origin_merge_requests .opened.where(source_branch: branch).to_a diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 52ce26a20b1..9ef88f233c5 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -32,6 +32,10 @@ %li = link_to "#shared", 'data-toggle' => 'tab' do Shared Projects + - if @nested_groups.present? + %li + = link_to "#groups", 'data-toggle' => 'tab' do + Subgroups .nav-controls = form_tag request.path, method: :get, class: 'project-filter-form', id: 'project-filter-form' do |f| = search_field_tag :filter_projects, nil, placeholder: 'Filter by name', class: 'projects-list-filter form-control', spellcheck: false @@ -47,3 +51,8 @@ - if @shared_projects.present? .tab-pane#shared = render "shared_projects", projects: @shared_projects + + - if @nested_groups.present? + .tab-pane#groups + %ul.content-list + = render partial: 'shared/groups/group', collection: @nested_groups diff --git a/app/views/layouts/nav/_dashboard.html.haml b/app/views/layouts/nav/_dashboard.html.haml index 817e4bebb05..205d23178d2 100644 --- a/app/views/layouts/nav/_dashboard.html.haml +++ b/app/views/layouts/nav/_dashboard.html.haml @@ -35,3 +35,7 @@ = link_to dashboard_snippets_path, title: 'Snippets' do %span Snippets + + = link_to help_path, title: 'About GitLab CE', class: 'about-gitlab' do + %span + About GitLab CE diff --git a/app/views/projects/branches/index.html.haml b/app/views/projects/branches/index.html.haml index 5fd664c7a93..c2a5e725efc 100644 --- a/app/views/projects/branches/index.html.haml +++ b/app/views/projects/branches/index.html.haml @@ -3,7 +3,7 @@ = render "projects/commits/head" %div{ class: container_class } - .top-area + .top-area.adjust .nav-text Protected branches can be managed in project settings @@ -12,7 +12,7 @@ = search_field_tag :search, params[:search], { placeholder: 'Filter by branch name', id: 'branch-search', class: 'form-control search-text-input input-short', spellcheck: false } .dropdown.inline - %button.dropdown-toggle{type: 'button', 'data-toggle' => 'dropdown'} + %button.dropdown-menu-toggle{type: 'button', 'data-toggle' => 'dropdown'} %span.light = projects_sort_options_hash[@sort] = icon('chevron-down') diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index 349181be784..34ead6427e0 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -10,7 +10,7 @@ %span.pull-right = link_to 'Change branches', mr_change_branches_path(@merge_request) %hr -= form_for [@project.namespace.becomes(Namespace), @project, @merge_request], html: { class: 'merge-request-form form-horizontal common-note-form js-requires-input' } do |f| += form_for [@project.namespace.becomes(Namespace), @project, @merge_request], html: { class: 'merge-request-form form-horizontal common-note-form js-requires-input js-quick-submit' } do |f| = render 'shared/issuable/form', f: f, issuable: @merge_request = f.hidden_field :source_project_id = f.hidden_field :source_branch diff --git a/app/views/projects/tags/index.html.haml b/app/views/projects/tags/index.html.haml index d52162a5de8..92a023a5c85 100644 --- a/app/views/projects/tags/index.html.haml +++ b/app/views/projects/tags/index.html.haml @@ -2,17 +2,17 @@ - page_title "Tags" = render "projects/commits/head" -%div{ class: container_class } - .top-area - .nav-text +%div.flex-list{ class: container_class } + .top-area.flex-row + .nav-text.row-main-content Tags give the ability to mark specific points in history as being important - .nav-controls + .nav-controls.row-fixed-content = form_tag(filter_tags_path, method: :get) do = search_field_tag :search, params[:search], { placeholder: 'Filter by tag name', id: 'tag-search', class: 'form-control search-text-input input-short', spellcheck: false } .dropdown - %button.dropdown-toggle.btn{ type: 'button', data: { toggle: 'dropdown'} } + %button.dropdown-toggle{ type: 'button', data: { toggle: 'dropdown'} } %span.light = projects_sort_options_hash[@sort] = icon('chevron-down') diff --git a/changelogs/unreleased/24820-buttons-in-the-branches-page-are-stacking-on-top-of-each-other-depending-on-the-selected-filter.yml b/changelogs/unreleased/24820-buttons-in-the-branches-page-are-stacking-on-top-of-each-other-depending-on-the-selected-filter.yml new file mode 100644 index 00000000000..fac7697ede1 --- /dev/null +++ b/changelogs/unreleased/24820-buttons-in-the-branches-page-are-stacking-on-top-of-each-other-depending-on-the-selected-filter.yml @@ -0,0 +1,4 @@ +--- +title: fix button layout issue on branches page +merge_request: 8074 +author: diff --git a/changelogs/unreleased/24876-page-jumps-to-wrong-position-when-clicking-a-comment-anchor.yml b/changelogs/unreleased/24876-page-jumps-to-wrong-position-when-clicking-a-comment-anchor.yml new file mode 100644 index 00000000000..c31c89dc4bc --- /dev/null +++ b/changelogs/unreleased/24876-page-jumps-to-wrong-position-when-clicking-a-comment-anchor.yml @@ -0,0 +1,4 @@ +--- +title: ensure permalinks scroll to correct position on multiple clicks +merge_request: 8046 +author: diff --git a/changelogs/unreleased/24941-login-tabs-border.yml b/changelogs/unreleased/24941-login-tabs-border.yml new file mode 100644 index 00000000000..b06c21ad71a --- /dev/null +++ b/changelogs/unreleased/24941-login-tabs-border.yml @@ -0,0 +1,4 @@ +--- +title: fix border in login session tabs +merge_request: 8346 +author: diff --git a/changelogs/unreleased/25829-update-username-button-remains-disabled-upon-failure.yml b/changelogs/unreleased/25829-update-username-button-remains-disabled-upon-failure.yml new file mode 100644 index 00000000000..c82bacd8bcd --- /dev/null +++ b/changelogs/unreleased/25829-update-username-button-remains-disabled-upon-failure.yml @@ -0,0 +1,4 @@ +--- +title: re-enable change username button after failure +merge_request: 8332 +author: diff --git a/changelogs/unreleased/26134-ctrl-enter-does-not-submit-a-new-merge-request.yml b/changelogs/unreleased/26134-ctrl-enter-does-not-submit-a-new-merge-request.yml new file mode 100644 index 00000000000..40183f8d3fa --- /dev/null +++ b/changelogs/unreleased/26134-ctrl-enter-does-not-submit-a-new-merge-request.yml @@ -0,0 +1,4 @@ +--- +title: Make CTRL+Enter submits a new merge request +merge_request: 8360 +author: Saad Shahd diff --git a/changelogs/unreleased/dz-nested-group-misc.yml b/changelogs/unreleased/dz-nested-group-misc.yml new file mode 100644 index 00000000000..9c9d0b1c644 --- /dev/null +++ b/changelogs/unreleased/dz-nested-group-misc.yml @@ -0,0 +1,4 @@ +--- +title: Show nested groups tab on group page +merge_request: 8308 +author: diff --git a/changelogs/unreleased/issue_22664.yml b/changelogs/unreleased/issue_22664.yml new file mode 100644 index 00000000000..18a8d9ec6be --- /dev/null +++ b/changelogs/unreleased/issue_22664.yml @@ -0,0 +1,4 @@ +--- +title: Check if user can read project before being assigned to issue +merge_request: +author: diff --git a/changelogs/unreleased/issue_25682.yml b/changelogs/unreleased/issue_25682.yml new file mode 100644 index 00000000000..a50138756ba --- /dev/null +++ b/changelogs/unreleased/issue_25682.yml @@ -0,0 +1,4 @@ +--- +title: Parse JIRA issue references even if Issue Tracker is disabled +merge_request: +author: diff --git a/changelogs/unreleased/mentioned-but-not-closed-issues-messages.yml b/changelogs/unreleased/mentioned-but-not-closed-issues-messages.yml new file mode 100644 index 00000000000..ba3b13bcdb7 --- /dev/null +++ b/changelogs/unreleased/mentioned-but-not-closed-issues-messages.yml @@ -0,0 +1,4 @@ +--- +title: Fix unclear closing issue behaviour on Merge Request show page +merge_request: 8345 +author: Gabriel Gizotti diff --git a/changelogs/unreleased/view-ce-vs-ee.yml b/changelogs/unreleased/view-ce-vs-ee.yml new file mode 100644 index 00000000000..38bce4ac7c3 --- /dev/null +++ b/changelogs/unreleased/view-ce-vs-ee.yml @@ -0,0 +1,4 @@ +--- +title: About GitLab link in sidebar that links to help page +merge_request: 8316 +author: diff --git a/db/migrate/20161227192806_rename_slack_and_mattermost_notification_services.rb b/db/migrate/20161227192806_rename_slack_and_mattermost_notification_services.rb new file mode 100644 index 00000000000..50ad7437227 --- /dev/null +++ b/db/migrate/20161227192806_rename_slack_and_mattermost_notification_services.rb @@ -0,0 +1,25 @@ +class RenameSlackAndMattermostNotificationServices < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + update_column_in_batches(:services, :type, 'SlackService') do |table, query| + query.where(table[:type].eq('SlackNotificationService')) + end + + update_column_in_batches(:services, :type, 'MattermostService') do |table, query| + query.where(table[:type].eq('MattermostNotificationService')) + end + end + + def down + update_column_in_batches(:services, :type, 'SlackNotificationService') do |table, query| + query.where(table[:type].eq('SlackService')) + end + + update_column_in_batches(:services, :type, 'MattermostNotificationService') do |table, query| + query.where(table[:type].eq('MattermostService')) + end + end +end diff --git a/db/schema.rb b/db/schema.rb index e1e72bdae8f..923ece86edb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20161226122833) do +ActiveRecord::Schema.define(version: 20161227192806) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" diff --git a/doc/api/settings.md b/doc/api/settings.md index 218546aafea..0bd38a6e664 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -79,6 +79,7 @@ PUT /application/settings | `enabled_git_access_protocol` | string | no | Enabled protocols for Git access. Allowed values are: `ssh`, `http`, and `nil` to allow both protocols. | | `koding_enabled` | boolean | no | Enable Koding integration. Default is `false`. | | `koding_url` | string | yes (if `koding_enabled` is `true`) | The Koding instance URL for integration. | +| `disabled_oauth_sign_in_sources` | Array of strings | no | Disabled OAuth sign-in sources | ```bash curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/application/settings?signup_enabled=false&default_project_visibility=1 diff --git a/lib/api/settings.rb b/lib/api/settings.rb index c4cb1c7924a..9eb9a105bde 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -9,23 +9,117 @@ module API end end - # Get current applicaiton settings - # - # Example Request: - # GET /application/settings + desc 'Get the current application settings' do + success Entities::ApplicationSetting + end get "application/settings" do present current_settings, with: Entities::ApplicationSetting end - # Modify application settings - # - # Example Request: - # PUT /application/settings + desc 'Modify application settings' do + success Entities::ApplicationSetting + end + params do + optional :default_branch_protection, type: Integer, values: [0, 1, 2], desc: 'Determine if developers can push to master' + optional :default_project_visibility, type: Integer, values: Gitlab::VisibilityLevel.values, desc: 'The default project visibility' + optional :default_snippet_visibility, type: Integer, values: Gitlab::VisibilityLevel.values, desc: 'The default snippet visibility' + optional :default_group_visibility, type: Integer, values: Gitlab::VisibilityLevel.values, desc: 'The default group visibility' + optional :restricted_visibility_levels, type: Array[String], desc: 'Selected levels cannot be used by non-admin users for projects or snippets. If the public level is restricted, user profiles are only visible to logged in users.' + optional :import_sources, type: Array[String], values: %w[github bitbucket gitlab google_code fogbugz git gitlab_project], + desc: 'Enabled sources for code import during project creation. OmniAuth must be configured for GitHub, Bitbucket, and GitLab.com' + optional :disabled_oauth_sign_in_sources, type: Array[String], desc: 'Disable certain OAuth sign-in sources' + optional :enabled_git_access_protocol, type: String, values: %w[ssh http nil], desc: 'Allow only the selected protocols to be used for Git access.' + optional :gravatar_enabled, type: Boolean, desc: 'Flag indicating if the Gravatar service is enabled' + optional :default_projects_limit, type: Integer, desc: 'The maximum number of personal projects' + optional :max_attachment_size, type: Integer, desc: 'Maximum attachment size in MB' + optional :session_expire_delay, type: Integer, desc: 'Session duration in minutes. GitLab restart is required to apply changes.' + optional :user_oauth_applications, type: Boolean, desc: 'Allow users to register any application to use GitLab as an OAuth provider' + optional :user_default_external, type: Boolean, desc: 'Newly registered users will by default be external' + optional :signup_enabled, type: Boolean, desc: 'Flag indicating if sign up is enabled' + optional :send_user_confirmation_email, type: Boolean, desc: 'Send confirmation email on sign-up' + optional :domain_whitelist, type: String, desc: 'ONLY users with e-mail addresses that match these domain(s) will be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' + optional :domain_blacklist_enabled, type: Boolean, desc: 'Enable domain blacklist for sign ups' + given domain_blacklist_enabled: ->(val) { val } do + requires :domain_blacklist, type: String, desc: 'Users with e-mail addresses that match these domain(s) will NOT be able to sign-up. Wildcards allowed. Use separate lines for multiple entries. Ex: domain.com, *.domain.com' + end + optional :after_sign_up_text, type: String, desc: 'Text shown after sign up' + optional :signin_enabled, type: Boolean, desc: 'Flag indicating if sign in is enabled' + optional :require_two_factor_authentication, type: Boolean, desc: 'Require all users to setup Two-factor authentication' + given require_two_factor_authentication: ->(val) { val } do + requires :two_factor_grace_period, type: Integer, desc: 'Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication' + end + optional :home_page_url, type: String, desc: 'We will redirect non-logged in users to this page' + optional :after_sign_out_path, type: String, desc: 'We will redirect users to this page after they sign out' + optional :sign_in_text, type: String, desc: 'The sign in text of the GitLab application' + optional :help_page_text, type: String, desc: 'Custom text displayed on the help page' + optional :shared_runners_enabled, type: Boolean, desc: 'Enable shared runners for new projects' + given shared_runners_enabled: ->(val) { val } do + requires :shared_runners_text, type: String, desc: 'Shared runners text ' + end + optional :max_artifacts_size, type: Integer, desc: "Set the maximum file size each build's artifacts can have" + optional :container_registry_token_expire_delay, type: Integer, desc: 'Authorization token duration (minutes)' + optional :metrics_enabled, type: Boolean, desc: 'Enable the InfluxDB metrics' + given metrics_enabled: ->(val) { val } do + requires :metrics_host, type: String, desc: 'The InfluxDB host' + requires :metrics_port, type: Integer, desc: 'The UDP port to use for connecting to InfluxDB' + requires :metrics_pool_size, type: Integer, desc: 'The amount of InfluxDB connections to open' + requires :metrics_timeout, type: Integer, desc: 'The amount of seconds after which an InfluxDB connection will time out' + requires :metrics_method_call_threshold, type: Integer, desc: 'A method call is only tracked when it takes longer to complete than the given amount of milliseconds.' + requires :metrics_sample_interval, type: Integer, desc: 'The sampling interval in seconds' + requires :metrics_packet_size, type: Integer, desc: 'The amount of points to store in a single UDP packet' + end + optional :sidekiq_throttling_enabled, type: Boolean, desc: 'Enable Sidekiq Job Throttling' + given sidekiq_throttling_enabled: ->(val) { val } do + requires :sidekiq_throttling_queus, type: Array[String], desc: 'Choose which queues you wish to throttle' + requires :sidekiq_throttling_factor, type: Float, desc: 'The factor by which the queues should be throttled. A value between 0.0 and 1.0, exclusive.' + end + optional :recaptcha_enabled, type: Boolean, desc: 'Helps prevent bots from creating accounts' + given recaptcha_enabled: ->(val) { val } do + requires :recaptcha_site_key, type: String, desc: 'Generate site key at http://www.google.com/recaptcha' + requires :recaptcha_private_key, type: String, desc: 'Generate private key at http://www.google.com/recaptcha' + end + optional :akismet_enabled, type: Boolean, desc: 'Helps prevent bots from creating issues' + given akismet_enabled: ->(val) { val } do + requires :akismet_api_key, type: String, desc: 'Generate API key at http://www.akismet.com' + end + optional :admin_notification_email, type: String, desc: 'Abuse reports will be sent to this address if it is set. Abuse reports are always available in the admin area.' + optional :sentry_enabled, type: Boolean, desc: 'Sentry is an error reporting and logging tool which is currently not shipped with GitLab, get it here: https://getsentry.com' + given sentry_enabled: ->(val) { val } do + requires :sentry_dsn, type: String, desc: 'Sentry Data Source Name' + end + optional :repository_storage, type: String, desc: 'Storage paths for new projects' + optional :repository_checks_enabled, type: Boolean, desc: "GitLab will periodically run 'git fsck' in all project and wiki repositories to look for silent disk corruption issues." + optional :koding_enabled, type: Boolean, desc: 'Enable Koding' + given koding_enabled: ->(val) { val } do + requires :koding_url, type: String, desc: 'The Koding team URL' + end + optional :version_check_enabled, type: Boolean, desc: 'Let GitLab inform you when an update is available.' + optional :email_author_in_body, type: Boolean, desc: 'Some email servers do not support overriding the email sender name. Enable this option to include the name of the author of the issue, merge request or comment in the email body instead.' + optional :html_emails_enabled, type: Boolean, desc: 'By default GitLab sends emails in HTML and plain text formats so mail clients can choose what format to use. Disable this option if you only want to send emails in plain text format.' + optional :housekeeping_enabled, type: Boolean, desc: 'Enable automatic repository housekeeping (git repack, git gc)' + given housekeeping_enabled: ->(val) { val } do + requires :housekeeping_bitmaps_enabled, type: Boolean, desc: "Creating pack file bitmaps makes housekeeping take a little longer but bitmaps should accelerate 'git clone' performance." + requires :housekeeping_incremental_repack_period, type: Integer, desc: "Number of Git pushes after which an incremental 'git repack' is run." + requires :housekeeping_full_repack_period, type: Integer, desc: "Number of Git pushes after which a full 'git repack' is run." + requires :housekeeping_gc_period, type: Integer, desc: "Number of Git pushes after which 'git gc' is run." + end + at_least_one_of :default_branch_protection, :default_project_visibility, :default_snippet_visibility, + :default_group_visibility, :restricted_visibility_levels, :import_sources, + :enabled_git_access_protocol, :gravatar_enabled, :default_projects_limit, + :max_attachment_size, :session_expire_delay, :disabled_oauth_sign_in_sources, + :user_oauth_applications, :user_default_external, :signup_enabled, + :send_user_confirmation_email, :domain_whitelist, :domain_blacklist_enabled, + :after_sign_up_text, :signin_enabled, :require_two_factor_authentication, + :home_page_url, :after_sign_out_path, :sign_in_text, :help_page_text, + :shared_runners_enabled, :max_artifacts_size, :container_registry_token_expire_delay, + :metrics_enabled, :sidekiq_throttling_enabled, :recaptcha_enabled, + :akismet_enabled, :admin_notification_email, :sentry_enabled, + :repository_storage, :repository_checks_enabled, :koding_enabled, + :version_check_enabled, :email_author_in_body, :html_emails_enabled, + :housekeeping_enabled + end put "application/settings" do - attributes = ["repository_storage"] + current_settings.attributes.keys - ["id"] - attrs = attributes_for_keys(attributes) - - if current_settings.update_attributes(attrs) + if current_settings.update_attributes(declared_params(include_missing: false)) present current_settings, with: Entities::ApplicationSetting else render_validation_error!(current_settings) diff --git a/spec/features/groups_spec.rb b/spec/features/groups_spec.rb index 4b19886274e..a515c92db37 100644 --- a/spec/features/groups_spec.rb +++ b/spec/features/groups_spec.rb @@ -107,4 +107,17 @@ feature 'Group', feature: true do expect(page).to have_css('.group-home-desc a[rel]') end end + + describe 'group page with nested groups', js: true do + let!(:group) { create(:group) } + let!(:nested_group) { create(:group, parent: group) } + let!(:path) { group_path(group) } + + it 'has nested groups tab with nested groups inside' do + visit path + click_link 'Subgroups' + + expect(page).to have_content(nested_group.full_name) + end + end end diff --git a/spec/features/milestones/milestones_spec.rb b/spec/features/milestones/milestones_spec.rb index 8b603f51545..aadd72a9f8e 100644 --- a/spec/features/milestones/milestones_spec.rb +++ b/spec/features/milestones/milestones_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe 'Milestone draggable', feature: true, js: true do + include WaitForAjax + let(:milestone) { create(:milestone, project: project, title: 8.14) } let(:project) { create(:empty_project, :public) } let(:user) { create(:user) } @@ -74,6 +76,8 @@ describe 'Milestone draggable', feature: true, js: true do visit namespace_project_milestone_path(project.namespace, project, milestone) issue.drag_to(issue_target) + + wait_for_ajax end def create_and_drag_merge_request(params = {}) @@ -82,5 +86,7 @@ describe 'Milestone draggable', feature: true, js: true do visit namespace_project_milestone_path(project.namespace, project, milestone) page.find("a[href='#tab-merge-requests']").click merge_request.drag_to(merge_request_target) + + wait_for_ajax end end diff --git a/spec/features/projects/features_visibility_spec.rb b/spec/features/projects/features_visibility_spec.rb index 3bb33394be7..9079350186d 100644 --- a/spec/features/projects/features_visibility_spec.rb +++ b/spec/features/projects/features_visibility_spec.rb @@ -42,6 +42,17 @@ describe 'Edit Project Settings', feature: true do end end + context "When external issue tracker is enabled" do + it "does not hide issues tab" do + project.project_feature.update(issues_access_level: ProjectFeature::DISABLED) + allow_any_instance_of(Project).to receive(:external_issue_tracker).and_return(JiraService.new) + + visit namespace_project_path(project.namespace, project) + + expect(page).to have_selector(".shortcuts-issues") + end + end + context "pipelines subtabs" do it "shows builds when enabled" do visit namespace_project_pipelines_path(project.namespace, project) diff --git a/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb index 50a5d1a19ba..0af36776a54 100644 --- a/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb @@ -12,7 +12,17 @@ describe Banzai::ReferenceParser::ExternalIssueParser, lib: true do context 'when the link has a data-issue attribute' do before { link['data-external-issue'] = 123 } - it_behaves_like "referenced feature visibility", "issues" + levels = [ProjectFeature::DISABLED, ProjectFeature::PRIVATE, ProjectFeature::ENABLED] + + levels.each do |level| + it "creates reference when the feature is #{level}" do + project.project_feature.update(issues_access_level: level) + + visible_nodes = subject.nodes_visible_to_user(user, [link]) + + expect(visible_nodes).to include(link) + end + end end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 4fa06a8c60a..1078c959419 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -44,6 +44,45 @@ describe Issue, "Issuable" do it { expect(described_class).to respond_to(:assigned) } end + describe "before_save" do + describe "#update_cache_counts" do + context "when previous assignee exists" do + before do + assignee = create(:user) + issue.project.team << [assignee, :developer] + issue.update(assignee: assignee) + end + + it "updates cache counts for new assignee" do + user = create(:user) + + expect(user).to receive(:update_cache_counts) + + issue.update(assignee: user) + end + + it "updates cache counts for previous assignee" do + old_assignee = issue.assignee + allow(User).to receive(:find_by_id).with(old_assignee.id).and_return(old_assignee) + + expect(old_assignee).to receive(:update_cache_counts) + + issue.update(assignee: nil) + end + end + + context "when previous assignee does not exist" do + before{ issue.update(assignee: nil) } + + it "updates cache count for the new assignee" do + expect_any_instance_of(User).to receive(:update_cache_counts) + + issue.update(assignee: user) + end + end + end + end + describe ".search" do let!(:searchable_issue) { create(:issue, title: "Searchable issue") } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 7d5ecfbaa64..45fe927202b 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -277,4 +277,15 @@ describe Group, models: true do it { is_expected.to be_valid } it { expect(subject.parent).to be_kind_of(Group) } end + + describe '#members_with_parents' do + let!(:group) { create(:group, :nested) } + let!(:master) { group.parent.add_user(create(:user), GroupMember::MASTER) } + let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) } + + it 'returns parents members' do + expect(group.members_with_parents).to include(developer) + expect(group.members_with_parents).to include(master) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 5da00a8636a..646e6c6dbb3 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -283,12 +283,16 @@ describe MergeRequest, models: true do end describe '#issues_mentioned_but_not_closing' do - it 'detects issues mentioned in description but not closed' do - mentioned_issue = create(:issue, project: subject.project) + let(:closing_issue) { create :issue, project: subject.project } + let(:mentioned_issue) { create :issue, project: subject.project } + + let(:commit) { double('commit', safe_message: "Fixes #{closing_issue.to_reference}") } + it 'detects issues mentioned in description but not closed' do subject.project.team << [subject.author, :developer] - subject.description = "Is related to #{mentioned_issue.to_reference}" + subject.description = "Is related to #{mentioned_issue.to_reference} and #{closing_issue.to_reference}" + allow(subject).to receive(:commits).and_return([commit]) allow(subject.project).to receive(:default_branch). and_return(subject.target_branch) diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index a20ac303a53..5c34ff04152 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -105,4 +105,70 @@ describe GroupPolicy, models: true do is_expected.to include(*owner_permissions) end end + + describe 'private nested group inherit permissions' do + let(:nested_group) { create(:group, :private, parent: group) } + + subject { described_class.abilities(current_user, nested_group).to_set } + + context 'with no user' do + let(:current_user) { nil } + + it do + is_expected.not_to include(:read_group) + is_expected.not_to include(*master_permissions) + is_expected.not_to include(*owner_permissions) + end + end + + context 'guests' do + let(:current_user) { guest } + + it do + is_expected.to include(:read_group) + is_expected.not_to include(*master_permissions) + is_expected.not_to include(*owner_permissions) + end + end + + context 'reporter' do + let(:current_user) { reporter } + + it do + is_expected.to include(:read_group) + is_expected.not_to include(*master_permissions) + is_expected.not_to include(*owner_permissions) + end + end + + context 'developer' do + let(:current_user) { developer } + + it do + is_expected.to include(:read_group) + is_expected.not_to include(*master_permissions) + is_expected.not_to include(*owner_permissions) + end + end + + context 'master' do + let(:current_user) { master } + + it do + is_expected.to include(:read_group) + is_expected.to include(*master_permissions) + is_expected.not_to include(*owner_permissions) + end + end + + context 'owner' do + let(:current_user) { owner } + + it do + is_expected.to include(:read_group) + is_expected.to include(*master_permissions) + is_expected.to include(*owner_permissions) + end + end + end end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 9a8d633d657..ad9d8a25af4 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -44,8 +44,7 @@ describe API::Settings, 'Settings', api: true do put api("/application/settings", admin), koding_enabled: true expect(response).to have_http_status(400) - expect(json_response['message']).to have_key('koding_url') - expect(json_response['message']['koding_url']).to include "can't be blank" + expect(json_response['error']).to eq('koding_url is missing') end end end diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb index 71a0b8e2a12..14717a7455d 100644 --- a/spec/services/groups/create_service_spec.rb +++ b/spec/services/groups/create_service_spec.rb @@ -1,11 +1,12 @@ require 'spec_helper' -describe Groups::CreateService, services: true do - let!(:user) { create(:user) } +describe Groups::CreateService, '#execute', services: true do + let!(:user) { create(:user) } let!(:group_params) { { path: "group_path", visibility_level: Gitlab::VisibilityLevel::PUBLIC } } - describe "execute" do - let!(:service) { described_class.new(user, group_params ) } + describe 'visibility level restrictions' do + let!(:service) { described_class.new(user, group_params) } + subject { service.execute } context "create groups without restricted visibility level" do @@ -14,7 +15,29 @@ describe Groups::CreateService, services: true do context "cannot create group with restricted visibility level" do before { allow_any_instance_of(ApplicationSetting).to receive(:restricted_visibility_levels).and_return([Gitlab::VisibilityLevel::PUBLIC]) } + it { is_expected.not_to be_persisted } end end + + describe 'creating subgroup' do + let!(:group) { create(:group) } + let!(:service) { described_class.new(user, group_params.merge(parent_id: group.id)) } + + subject { service.execute } + + context 'as group owner' do + before { group.add_owner(user) } + + it { is_expected.to be_persisted } + end + + context 'as guest' do + it 'does not save group and returns an error' do + is_expected.not_to be_persisted + expect(subject.errors[:parent_id].first).to eq('manage access required to create subgroup') + expect(subject.parent_id).to be_nil + end + end + end end diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index 5f3020b6525..0475f38fe5e 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -52,7 +52,10 @@ describe Issuable::BulkUpdateService, services: true do context 'when the new assignee ID is a valid user' do it 'succeeds' do - result = bulk_update(issue, assignee_id: create(:user).id) + new_assignee = create(:user) + project.team << [new_assignee, :developer] + + result = bulk_update(issue, assignee_id: new_assignee.id) expect(result[:success]).to be_truthy expect(result[:count]).to eq(1) @@ -60,15 +63,16 @@ describe Issuable::BulkUpdateService, services: true do it 'updates the assignee to the use ID passed' do assignee = create(:user) + project.team << [assignee, :developer] expect { bulk_update(issue, assignee_id: assignee.id) } .to change { issue.reload.assignee }.from(user).to(assignee) end end - context 'when the new assignee ID is -1' do - it 'unassigns the issues' do - expect { bulk_update(issue, assignee_id: -1) } + context "when the new assignee ID is #{IssuableFinder::NONE}" do + it "unassigns the issues" do + expect { bulk_update(issue, assignee_id: IssuableFinder::NONE) } .to change { issue.reload.assignee }.to(nil) end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 8bde61ee336..ac3834c32ff 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -135,6 +135,8 @@ describe Issues::CreateService, services: true do end end + it_behaves_like 'issuable create service' + it_behaves_like 'new issuable record that supports slash commands' context 'for a merge request' do diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index eafbea46905..d83b09fd32c 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -142,6 +142,17 @@ describe Issues::UpdateService, services: true do update_issue(confidential: true) end + + it 'does not update assignee_id with unauthorized users' do + project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + update_issue(confidential: true) + non_member = create(:user) + original_assignee = issue.assignee + + update_issue(assignee_id: non_member.id) + + expect(issue.reload.assignee_id).to eq(original_assignee.id) + end end context 'todos' do diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index b8142889075..673c0bd6c9c 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -84,6 +84,8 @@ describe MergeRequests::CreateService, services: true do end end + it_behaves_like 'issuable create service' + context 'while saving references to issues that the created merge request closes' do let(:first_issue) { create(:issue, project: project) } let(:second_issue) { create(:issue, project: project) } diff --git a/spec/services/notes/slash_commands_service_spec.rb b/spec/services/notes/slash_commands_service_spec.rb index d1099884a02..960b5cd5e6f 100644 --- a/spec/services/notes/slash_commands_service_spec.rb +++ b/spec/services/notes/slash_commands_service_spec.rb @@ -5,6 +5,8 @@ describe Notes::SlashCommandsService, services: true do let(:project) { create(:empty_project) } let(:master) { create(:user).tap { |u| project.team << [u, :master] } } let(:assignee) { create(:user) } + + before { project.team << [assignee, :master] } end shared_examples 'note on noteable that does not support slash commands' do diff --git a/spec/support/services/issuable_create_service_shared_examples.rb b/spec/support/services/issuable_create_service_shared_examples.rb new file mode 100644 index 00000000000..93c0267d2db --- /dev/null +++ b/spec/support/services/issuable_create_service_shared_examples.rb @@ -0,0 +1,52 @@ +shared_examples 'issuable create service' do + context 'asssignee_id' do + let(:assignee) { create(:user) } + + before { project.team << [user, :master] } + + it 'removes assignee_id when user id is invalid' do + opts = { title: 'Title', description: 'Description', assignee_id: -1 } + + issuable = described_class.new(project, user, opts).execute + + expect(issuable.assignee_id).to be_nil + end + + it 'removes assignee_id when user id is 0' do + opts = { title: 'Title', description: 'Description', assignee_id: 0 } + + issuable = described_class.new(project, user, opts).execute + + expect(issuable.assignee_id).to be_nil + end + + it 'saves assignee when user id is valid' do + project.team << [assignee, :master] + opts = { title: 'Title', description: 'Description', assignee_id: assignee.id } + + issuable = described_class.new(project, user, opts).execute + + expect(issuable.assignee_id).to eq(assignee.id) + end + + context "when issuable feature is private" do + before do + project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE) + project.project_feature.update(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] + + levels.each do |level| + it "removes not authorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do + project.update(visibility_level: level) + opts = { title: 'Title', description: 'Description', assignee_id: assignee.id } + + issuable = described_class.new(project, user, opts).execute + + expect(issuable.assignee_id).to be_nil + end + end + end + end +end diff --git a/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb b/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb index 5f9645ed44f..dd54b0addda 100644 --- a/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb +++ b/spec/support/services/issuable_create_service_slash_commands_shared_examples.rb @@ -11,6 +11,8 @@ shared_examples 'new issuable record that supports slash commands' do let(:params) { base_params.merge(defined?(default_params) ? default_params : {}).merge(example_params) } let(:issuable) { described_class.new(project, user, params).execute } + before { project.team << [assignee, :master ] } + context 'with labels in command only' do let(:example_params) do { @@ -55,7 +57,7 @@ shared_examples 'new issuable record that supports slash commands' do context 'with assignee and milestone in params and command' do let(:example_params) do { - assignee: build_stubbed(:user), + assignee: create(:user), milestone_id: double(:milestone), description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}") } diff --git a/spec/support/services/issuable_update_service_shared_examples.rb b/spec/support/services/issuable_update_service_shared_examples.rb index a3336755773..49cea1e608c 100644 --- a/spec/support/services/issuable_update_service_shared_examples.rb +++ b/spec/support/services/issuable_update_service_shared_examples.rb @@ -1,4 +1,8 @@ shared_examples 'issuable update service' do + def update_issuable(opts) + described_class.new(project, user, opts).execute(open_issuable) + end + context 'changing state' do before { expect(project).to receive(:execute_hooks).once } @@ -14,4 +18,52 @@ shared_examples 'issuable update service' do end end end + + context 'asssignee_id' do + it 'does not update assignee when assignee_id is invalid' do + open_issuable.update(assignee_id: user.id) + + update_issuable(assignee_id: -1) + + expect(open_issuable.reload.assignee).to eq(user) + end + + it 'unassigns assignee when user id is 0' do + open_issuable.update(assignee_id: user.id) + + update_issuable(assignee_id: 0) + + expect(open_issuable.assignee_id).to be_nil + end + + it 'saves assignee when user id is valid' do + update_issuable(assignee_id: user.id) + + expect(open_issuable.assignee_id).to eq(user.id) + end + + it 'does not update assignee_id when user cannot read issue' do + non_member = create(:user) + original_assignee = open_issuable.assignee + + update_issuable(assignee_id: non_member.id) + + expect(open_issuable.assignee_id).to eq(original_assignee.id) + end + + context "when issuable feature is private" do + levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC] + + levels.each do |level| + it "does not update with unauthorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do + assignee = create(:user) + project.update(visibility_level: level) + feature_visibility_attr = :"#{open_issuable.model_name.plural}_access_level" + project.project_feature.update_attribute(feature_visibility_attr, ProjectFeature::PRIVATE) + + expect{ update_issuable(assignee_id: assignee) }.not_to change{ open_issuable.assignee } + end + end + end + end end |