diff options
author | Rémy Coutable <remy@rymai.me> | 2016-09-05 17:23:32 +0200 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-09-05 17:23:32 +0200 |
commit | 8aa025bb85cab3070a876b43d9c5462af5f35e67 (patch) | |
tree | ee0a11ec654dcf337b6badad796068f0bd54085e | |
parent | 89af76edc5e44ad1a0a55a65337bb992355911a6 (diff) | |
parent | a24822104a1b27560c8d3c76462ef778144f9491 (diff) | |
download | gitlab-ce-8aa025bb85cab3070a876b43d9c5462af5f35e67.tar.gz |
Merge branch 'master' of https://dev.gitlab.org/gitlab/gitlabhq
Signed-off-by: Rémy Coutable <remy@rymai.me>
36 files changed, 424 insertions, 186 deletions
diff --git a/CHANGELOG b/CHANGELOG index ae6812612d2..bc18171d091 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -100,6 +100,10 @@ v 8.11.4 - Creating an issue through our API now emails label subscribers !5720 - Block concurrent updates for Pipeline - Don't create groups for unallowed users when importing projects + - Fix resolving conflicts on forks + - Fix diff commenting on merge requests created prior to 8.10 + - Don't create groups for unallowed users when importing projects + - Scope webhooks/services that will run for confidential issues - Fix issue boards leak private label names and descriptions - Fix broken gitlab:backup:restore because of bad permissions on repo storage !6098 (Dirk Hörner) - Remove gitorious. !5866 diff --git a/app/assets/javascripts/importer_status.js b/app/assets/javascripts/importer_status.js index 0f840821f53..9efad1ce943 100644 --- a/app/assets/javascripts/importer_status.js +++ b/app/assets/javascripts/importer_status.js @@ -10,21 +10,24 @@ ImporterStatus.prototype.initStatusPage = function() { $('.js-add-to-import').off('click').on('click', (function(_this) { return function(e) { - var $btn, $namespace_input, $target_field, $tr, id, new_namespace; + var $btn, $namespace_input, $target_field, $tr, id, target_namespace; $btn = $(e.currentTarget); $tr = $btn.closest('tr'); $target_field = $tr.find('.import-target'); $namespace_input = $target_field.find('input'); id = $tr.attr('id').replace('repo_', ''); - new_namespace = null; + target_namespace = null; + if ($namespace_input.length > 0) { - new_namespace = $namespace_input.prop('value'); - $target_field.empty().append(new_namespace + "/" + ($target_field.data('project_name'))); + target_namespace = $namespace_input.prop('value'); + $target_field.empty().append(target_namespace + "/" + ($target_field.data('project_name'))); } + $btn.disable().addClass('is-loading'); + return $.post(_this.import_url, { repo_id: id, - new_namespace: new_namespace + target_namespace: target_namespace }, { dataType: 'script' }); @@ -70,7 +73,7 @@ if ($('.js-importer-status').length) { var jobsImportPath = $('.js-importer-status').data('jobs-import-path'); var importPath = $('.js-importer-status').data('import-path'); - + new ImporterStatus(jobsImportPath, importPath); } }); diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb index a69877edfd4..4cb3be41064 100644 --- a/app/controllers/concerns/service_params.rb +++ b/app/controllers/concerns/service_params.rb @@ -13,7 +13,7 @@ module ServiceParams # `issue_events` and `merge_request_events` (singular!) # See app/helpers/services_helper.rb for how we # make those event names plural as special case. - :issues_events, :merge_requests_events, + :issues_events, :confidential_issues_events, :merge_requests_events, :notify_only_broken_builds, :notify_only_broken_pipelines, :add_pusher, :send_from_committer_email, :disable_diffs, :external_wiki_url, :notify, :color, diff --git a/app/controllers/import/base_controller.rb b/app/controllers/import/base_controller.rb index 7e8597a5eb3..256c41e6145 100644 --- a/app/controllers/import/base_controller.rb +++ b/app/controllers/import/base_controller.rb @@ -1,18 +1,17 @@ class Import::BaseController < ApplicationController private - def get_or_create_namespace + def find_or_create_namespace(name, owner) + return current_user.namespace if name == owner + return current_user.namespace unless current_user.can_create_group? + begin - namespace = Group.create!(name: @target_namespace, path: @target_namespace, owner: current_user) + name = params[:target_namespace].presence || name + namespace = Group.create!(name: name, path: name, owner: current_user) namespace.add_owner(current_user) + namespace rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid - namespace = Namespace.find_by_path_or_name(@target_namespace) - unless current_user.can?(:create_projects, namespace) - @already_been_taken = true - return false - end + Namespace.find_by_path_or_name(name) end - - namespace end end diff --git a/app/controllers/import/bitbucket_controller.rb b/app/controllers/import/bitbucket_controller.rb index 944c73d139a..6ea54744da8 100644 --- a/app/controllers/import/bitbucket_controller.rb +++ b/app/controllers/import/bitbucket_controller.rb @@ -35,23 +35,20 @@ class Import::BitbucketController < Import::BaseController end def create - @repo_id = params[:repo_id] || "" - repo = client.project(@repo_id.gsub("___", "/")) - @project_name = repo["slug"] - - repo_owner = repo["owner"] - repo_owner = current_user.username if repo_owner == client.user["user"]["username"] - @target_namespace = params[:new_namespace].presence || repo_owner - - namespace = get_or_create_namespace || (render and return) + @repo_id = params[:repo_id].to_s + repo = client.project(@repo_id.gsub('___', '/')) + @project_name = repo['slug'] + @target_namespace = find_or_create_namespace(repo['owner'], client.user['user']['username']) unless Gitlab::BitbucketImport::KeyAdder.new(repo, current_user, access_params).execute - @access_denied = true - render - return + render 'deploy_key' and return end - @project = Gitlab::BitbucketImport::ProjectCreator.new(repo, namespace, current_user, access_params).execute + if current_user.can?(:create_projects, @target_namespace) + @project = Gitlab::BitbucketImport::ProjectCreator.new(repo, @target_namespace, current_user, access_params).execute + else + render 'unauthorized' + end end private diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index 9c1b0eb20f4..8c6bdd16383 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -41,14 +41,13 @@ class Import::GithubController < Import::BaseController @repo_id = params[:repo_id].to_i repo = client.repo(@repo_id) @project_name = repo.name + @target_namespace = find_or_create_namespace(repo.owner.login, client.user.login) - repo_owner = repo.owner.login - repo_owner = current_user.username if repo_owner == client.user.login - @target_namespace = params[:new_namespace].presence || repo_owner - - namespace = get_or_create_namespace || (render and return) - - @project = Gitlab::GithubImport::ProjectCreator.new(repo, namespace, current_user, access_params).execute + if current_user.can?(:create_projects, @target_namespace) + @project = Gitlab::GithubImport::ProjectCreator.new(repo, @target_namespace, current_user, access_params).execute + else + render 'unauthorized' + end end private diff --git a/app/controllers/import/gitlab_controller.rb b/app/controllers/import/gitlab_controller.rb index 08130ee8176..73837ffbe67 100644 --- a/app/controllers/import/gitlab_controller.rb +++ b/app/controllers/import/gitlab_controller.rb @@ -26,15 +26,14 @@ class Import::GitlabController < Import::BaseController def create @repo_id = params[:repo_id].to_i repo = client.project(@repo_id) - @project_name = repo["name"] + @project_name = repo['name'] + @target_namespace = find_or_create_namespace(repo['namespace']['path'], client.user['username']) - repo_owner = repo["namespace"]["path"] - repo_owner = current_user.username if repo_owner == client.user["username"] - @target_namespace = params[:new_namespace].presence || repo_owner - - namespace = get_or_create_namespace || (render and return) - - @project = Gitlab::GitlabImport::ProjectCreator.new(repo, namespace, current_user, access_params).execute + if current_user.can?(:create_projects, @target_namespace) + @project = Gitlab::GitlabImport::ProjectCreator.new(repo, @target_namespace, current_user, access_params).execute + else + render 'unauthorized' + end end private diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index b5624046387..0ae8ff98009 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -59,6 +59,7 @@ class Projects::HooksController < Projects::ApplicationController :pipeline_events, :enable_ssl_verification, :issues_events, + :confidential_issues_events, :merge_requests_events, :note_events, :push_events, diff --git a/app/helpers/import_helper.rb b/app/helpers/import_helper.rb index 109bc1a02d1..021d2b14718 100644 --- a/app/helpers/import_helper.rb +++ b/app/helpers/import_helper.rb @@ -1,4 +1,9 @@ module ImportHelper + def import_project_target(owner, name) + namespace = current_user.can_create_group? ? owner : current_user.namespace_path + "#{namespace}/#{name}" + end + def github_project_link(path_with_namespace) link_to path_with_namespace, github_project_url(path_with_namespace), target: '_blank' end diff --git a/app/helpers/services_helper.rb b/app/helpers/services_helper.rb index 2dd0bf5d71e..3d4abf76419 100644 --- a/app/helpers/services_helper.rb +++ b/app/helpers/services_helper.rb @@ -8,7 +8,9 @@ module ServicesHelper when "note" "Event will be triggered when someone adds a comment" when "issue" - "Event will be triggered when an issue is created/updated/merged" + "Event will be triggered when an issue is created/updated/closed" + when "confidential_issue" + "Event will be triggered when a confidential issue is created/updated/closed" when "merge_request" "Event will be triggered when a merge request is created/updated/merged" when "build" @@ -19,7 +21,7 @@ module ServicesHelper end def service_event_field_name(event) - event = event.pluralize if %w[merge_request issue].include?(event) + event = event.pluralize if %w[merge_request issue confidential_issue].include?(event) "#{event}_events" end end diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index 836a75b0608..c631e7a7df5 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -2,6 +2,7 @@ class ProjectHook < WebHook belongs_to :project scope :issue_hooks, -> { where(issues_events: true) } + scope :confidential_issue_hooks, -> { where(confidential_issues_events: true) } scope :note_hooks, -> { where(note_events: true) } scope :merge_request_hooks, -> { where(merge_requests_events: true) } scope :build_hooks, -> { where(build_events: true) } diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index f365dee3141..595602e80fe 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -4,6 +4,7 @@ class WebHook < ActiveRecord::Base default_value_for :push_events, true default_value_for :issues_events, false + default_value_for :confidential_issues_events, false default_value_for :note_events, false default_value_for :merge_requests_events, false default_value_for :tag_push_events, false diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index d7c986c1a91..afebd3b6a12 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -39,7 +39,7 @@ class HipchatService < Service end def supported_events - %w(push issue merge_request note tag_push build) + %w(push issue confidential_issue merge_request note tag_push build) end def execute(data) diff --git a/app/models/project_services/slack_service.rb b/app/models/project_services/slack_service.rb index abbc780dc1a..e6c943db2bf 100644 --- a/app/models/project_services/slack_service.rb +++ b/app/models/project_services/slack_service.rb @@ -44,7 +44,7 @@ class SlackService < Service end def supported_events - %w(push issue merge_request note tag_push build wiki_page) + %w(push issue confidential_issue merge_request note tag_push build wiki_page) end def execute(data) diff --git a/app/models/service.rb b/app/models/service.rb index 09b4717a523..198e7247838 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -7,6 +7,7 @@ class Service < ActiveRecord::Base default_value_for :active, false default_value_for :push_events, true default_value_for :issues_events, true + default_value_for :confidential_issues_events, true default_value_for :merge_requests_events, true default_value_for :tag_push_events, true default_value_for :note_events, true @@ -33,6 +34,7 @@ class Service < ActiveRecord::Base scope :push_hooks, -> { where(push_events: true, active: true) } scope :tag_push_hooks, -> { where(tag_push_events: true, active: true) } scope :issue_hooks, -> { where(issues_events: true, active: true) } + scope :confidential_issue_hooks, -> { where(confidential_issues_events: true, active: true) } scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) } scope :note_hooks, -> { where(note_events: true, active: true) } scope :build_hooks, -> { where(build_events: true, active: true) } @@ -100,7 +102,7 @@ class Service < ActiveRecord::Base end def supported_events - %w(push tag_push issue merge_request wiki_page) + %w(push tag_push issue confidential_issue merge_request wiki_page) end def execute(data) diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 089b0f527e2..9ea3ce084ba 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -14,9 +14,10 @@ module Issues end def execute_hooks(issue, action = 'open') - issue_data = hook_data(issue, action) - issue.project.execute_hooks(issue_data, :issue_hooks) - issue.project.execute_services(issue_data, :issue_hooks) + issue_data = hook_data(issue, action) + hooks_scope = issue.confidential? ? :confidential_issue_hooks : :issue_hooks + issue.project.execute_hooks(issue_data, hooks_scope) + issue.project.execute_services(issue_data, hooks_scope) end end end diff --git a/app/views/import/base/create.js.haml b/app/views/import/base/create.js.haml index 804ad88468f..8e929538351 100644 --- a/app/views/import/base/create.js.haml +++ b/app/views/import/base/create.js.haml @@ -1,23 +1,4 @@ -- if @already_been_taken - :plain - tr = $("tr#repo_#{@repo_id}") - target_field = tr.find(".import-target") - import_button = tr.find(".btn-import") - origin_target = target_field.text() - project_name = "#{@project_name}" - origin_namespace = "#{@target_namespace}" - target_field.empty() - target_field.append("<p class='alert alert-danger'>This namespace already been taken! Please choose another one</p>") - target_field.append("<input type='text' name='target_namespace' />") - target_field.append("/" + project_name) - target_field.data("project_name", project_name) - target_field.find('input').prop("value", origin_namespace) - import_button.enable().removeClass('is-loading') -- elsif @access_denied - :plain - job = $("tr#repo_#{@repo_id}") - job.find(".import-actions").html("<p class='alert alert-danger'>Access denied! Please verify you can add deploy keys to this repository.</p>") -- elsif @project.persisted? +- if @project.persisted? :plain job = $("tr#repo_#{@repo_id}") job.attr("id", "project_#{@project.id}") diff --git a/app/views/import/base/unauthorized.js.haml b/app/views/import/base/unauthorized.js.haml new file mode 100644 index 00000000000..36f8069c1f7 --- /dev/null +++ b/app/views/import/base/unauthorized.js.haml @@ -0,0 +1,14 @@ +:plain + tr = $("tr#repo_#{@repo_id}") + target_field = tr.find(".import-target") + import_button = tr.find(".btn-import") + origin_target = target_field.text() + project_name = "#{@project_name}" + origin_namespace = "#{@target_namespace.path}" + target_field.empty() + target_field.append("<p class='alert alert-danger'>This namespace has already been taken! Please choose another one.</p>") + target_field.append("<input type='text' name='target_namespace' />") + target_field.append("/" + project_name) + target_field.data("project_name", project_name) + target_field.find('input').prop("value", origin_namespace) + import_button.enable().removeClass('is-loading') diff --git a/app/views/import/bitbucket/deploy_key.js.haml b/app/views/import/bitbucket/deploy_key.js.haml new file mode 100644 index 00000000000..81b34ab5c9d --- /dev/null +++ b/app/views/import/bitbucket/deploy_key.js.haml @@ -0,0 +1,3 @@ +:plain + job = $("tr#repo_#{@repo_id}") + job.find(".import-actions").html("<p class='alert alert-danger'>Access denied! Please verify you can add deploy keys to this repository.</p>") diff --git a/app/views/import/bitbucket/status.html.haml b/app/views/import/bitbucket/status.html.haml index 15dd98077c8..f8b4b107513 100644 --- a/app/views/import/bitbucket/status.html.haml +++ b/app/views/import/bitbucket/status.html.haml @@ -51,7 +51,7 @@ %td = link_to "#{repo["owner"]}/#{repo["slug"]}", "https://bitbucket.org/#{repo["owner"]}/#{repo["slug"]}", target: "_blank" %td.import-target - = "#{repo["owner"]}/#{repo["slug"]}" + = import_project_target(repo['owner'], repo['slug']) %td.import-actions.job-status = button_tag class: "btn btn-import js-add-to-import" do Import diff --git a/app/views/import/github/status.html.haml b/app/views/import/github/status.html.haml index 54ff1d27c67..bd3be20c4f8 100644 --- a/app/views/import/github/status.html.haml +++ b/app/views/import/github/status.html.haml @@ -45,7 +45,7 @@ %td = github_project_link(repo.full_name) %td.import-target - = repo.full_name + = import_project_target(repo.owner.login, repo.name) %td.import-actions.job-status = button_tag class: "btn btn-import js-add-to-import" do Import diff --git a/app/views/import/gitlab/status.html.haml b/app/views/import/gitlab/status.html.haml index fcfc6fd37f4..d31fc2e6adb 100644 --- a/app/views/import/gitlab/status.html.haml +++ b/app/views/import/gitlab/status.html.haml @@ -45,7 +45,7 @@ %td = link_to repo["path_with_namespace"], "https://gitlab.com/#{repo["path_with_namespace"]}", target: "_blank" %td.import-target - = repo["path_with_namespace"] + = import_project_target(repo['namespace']['path'], repo['name']) %td.import-actions.job-status = button_tag class: "btn btn-import js-add-to-import" do Import diff --git a/app/views/projects/hooks/_project_hook.html.haml b/app/views/projects/hooks/_project_hook.html.haml index 3fcf1692e09..ceabe2eab3d 100644 --- a/app/views/projects/hooks/_project_hook.html.haml +++ b/app/views/projects/hooks/_project_hook.html.haml @@ -3,7 +3,7 @@ .col-md-8.col-lg-7 %strong.light-header= hook.url %div - - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events pipeline_events wiki_page_events).each do |trigger| + - %w(push_events tag_push_events issues_events confidential_issues_events note_events merge_requests_events build_events pipeline_events wiki_page_events).each do |trigger| - if hook.send(trigger) %span.label.label-gray.deploy-project-label= trigger.titleize .col-md-4.col-lg-5.text-right-lg.prepend-top-5 diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml index d2ec6c3ddef..5d659eb83a9 100644 --- a/app/views/shared/web_hooks/_form.html.haml +++ b/app/views/shared/web_hooks/_form.html.haml @@ -52,6 +52,13 @@ %p.light This URL will be triggered when an issue is created/updated/merged %li + = f.check_box :confidential_issues_events, class: 'pull-left' + .prepend-left-20 + = f.label :confidential_issues_events, class: 'list-label' do + %strong Confidential Issues events + %p.light + This URL will be triggered when a confidential issue is created/updated/merged + %li = f.check_box :merge_requests_events, class: 'pull-left' .prepend-left-20 = f.label :merge_requests_events, class: 'list-label' do diff --git a/db/migrate/20160830203109_add_confidential_issues_events_to_web_hooks.rb b/db/migrate/20160830203109_add_confidential_issues_events_to_web_hooks.rb new file mode 100644 index 00000000000..a27947212f6 --- /dev/null +++ b/db/migrate/20160830203109_add_confidential_issues_events_to_web_hooks.rb @@ -0,0 +1,15 @@ +class AddConfidentialIssuesEventsToWebHooks < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default :web_hooks, :confidential_issues_events, :boolean, default: false, allow_null: false + end + + def down + remove_column :web_hooks, :confidential_issues_events + end +end diff --git a/db/migrate/20160830211132_add_confidential_issues_events_to_services.rb b/db/migrate/20160830211132_add_confidential_issues_events_to_services.rb new file mode 100644 index 00000000000..030e7c39350 --- /dev/null +++ b/db/migrate/20160830211132_add_confidential_issues_events_to_services.rb @@ -0,0 +1,15 @@ +class AddConfidentialIssuesEventsToServices < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default :services, :confidential_issues_events, :boolean, default: true, allow_null: false + end + + def down + remove_column :services, :confidential_issues_events + end +end diff --git a/db/migrate/20160901141443_set_confidential_issues_events_on_webhooks.rb b/db/migrate/20160901141443_set_confidential_issues_events_on_webhooks.rb new file mode 100644 index 00000000000..f1a1f001cb3 --- /dev/null +++ b/db/migrate/20160901141443_set_confidential_issues_events_on_webhooks.rb @@ -0,0 +1,15 @@ +class SetConfidentialIssuesEventsOnWebhooks < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + update_column_in_batches(:web_hooks, :confidential_issues_events, true) do |table, query| + query.where(table[:issues_events].eq(true)) + end + end + + def down + # noop + end +end diff --git a/db/schema.rb b/db/schema.rb index af6e74a4e25..c9023a02c77 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: 20160831223750) do +ActiveRecord::Schema.define(version: 20160901141443) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -909,19 +909,20 @@ ActiveRecord::Schema.define(version: 20160831223750) do t.integer "project_id" t.datetime "created_at" t.datetime "updated_at" - t.boolean "active", default: false, null: false + t.boolean "active", default: false, null: false t.text "properties" - t.boolean "template", default: false - t.boolean "push_events", default: true - t.boolean "issues_events", default: true - t.boolean "merge_requests_events", default: true - t.boolean "tag_push_events", default: true - t.boolean "note_events", default: true, null: false - t.boolean "build_events", default: false, null: false - t.string "category", default: "common", null: false - t.boolean "default", default: false - t.boolean "wiki_page_events", default: true - t.boolean "pipeline_events", default: false, null: false + t.boolean "template", default: false + t.boolean "push_events", default: true + t.boolean "issues_events", default: true + t.boolean "merge_requests_events", default: true + t.boolean "tag_push_events", default: true + t.boolean "note_events", default: true, null: false + t.boolean "build_events", default: false, null: false + t.string "category", default: "common", null: false + t.boolean "default", default: false + t.boolean "wiki_page_events", default: true + t.boolean "pipeline_events", default: false, null: false + t.boolean "confidential_issues_events", default: true, null: false end add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree @@ -1121,22 +1122,23 @@ ActiveRecord::Schema.define(version: 20160831223750) do add_index "users_star_projects", ["user_id"], name: "index_users_star_projects_on_user_id", using: :btree create_table "web_hooks", force: :cascade do |t| - t.string "url", limit: 2000 + t.string "url", limit: 2000 t.integer "project_id" t.datetime "created_at" t.datetime "updated_at" - t.string "type", default: "ProjectHook" + t.string "type", default: "ProjectHook" t.integer "service_id" - t.boolean "push_events", default: true, null: false - t.boolean "issues_events", default: false, null: false - t.boolean "merge_requests_events", default: false, null: false - t.boolean "tag_push_events", default: false - t.boolean "note_events", default: false, null: false - t.boolean "enable_ssl_verification", default: true - t.boolean "build_events", default: false, null: false - t.boolean "wiki_page_events", default: false, null: false + t.boolean "push_events", default: true, null: false + t.boolean "issues_events", default: false, null: false + t.boolean "merge_requests_events", default: false, null: false + t.boolean "tag_push_events", default: false + t.boolean "note_events", default: false, null: false + t.boolean "enable_ssl_verification", default: true + t.boolean "build_events", default: false, null: false + t.boolean "wiki_page_events", default: false, null: false t.string "token" - t.boolean "pipeline_events", default: false, null: false + t.boolean "pipeline_events", default: false, null: false + t.boolean "confidential_issues_events", default: false, null: false end add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb index 07bf8d2d1c3..1d3c9fbbe2f 100644 --- a/spec/controllers/import/bitbucket_controller_spec.rb +++ b/spec/controllers/import/bitbucket_controller_spec.rb @@ -146,21 +146,42 @@ describe Import::BitbucketController do end context "when a namespace with the Bitbucket user's username doesn't exist" do - it "creates the namespace" do - expect(Gitlab::BitbucketImport::ProjectCreator). - to receive(:new).and_return(double(execute: true)) + context "when current user can create namespaces" do + it "creates the namespace" do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).and_return(double(execute: true)) - post :create, format: :js + expect { post :create, format: :js }.to change(Namespace, :count).by(1) + end + + it "takes the new namespace" do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, an_instance_of(Group), user, access_params). + and_return(double(execute: true)) - expect(Namespace.where(name: other_username).first).not_to be_nil + post :create, format: :js + end end - it "takes the new namespace" do - expect(Gitlab::BitbucketImport::ProjectCreator). - to receive(:new).with(bitbucket_repo, an_instance_of(Group), user, access_params). - and_return(double(execute: true)) + context "when current user can't create namespaces" do + before do + user.update_attribute(:can_create_group, false) + end - post :create, format: :js + it "doesn't create the namespace" do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).and_return(double(execute: true)) + + expect { post :create, format: :js }.not_to change(Namespace, :count) + end + + it "takes the current user's namespace" do + expect(Gitlab::BitbucketImport::ProjectCreator). + to receive(:new).with(bitbucket_repo, user.namespace, user, access_params). + and_return(double(execute: true)) + + post :create, format: :js + end end end end diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 51d59526854..ebfbf54182b 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -181,21 +181,42 @@ describe Import::GithubController do end context "when a namespace with the GitHub user's username doesn't exist" do - it "creates the namespace" do - expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).and_return(double(execute: true)) + context "when current user can create namespaces" do + it "creates the namespace" do + expect(Gitlab::GithubImport::ProjectCreator). + to receive(:new).and_return(double(execute: true)) - post :create, format: :js + expect { post :create, format: :js }.to change(Namespace, :count).by(1) + end + + it "takes the new namespace" do + expect(Gitlab::GithubImport::ProjectCreator). + to receive(:new).with(github_repo, an_instance_of(Group), user, access_params). + and_return(double(execute: true)) - expect(Namespace.where(name: other_username).first).not_to be_nil + post :create, format: :js + end end - it "takes the new namespace" do - expect(Gitlab::GithubImport::ProjectCreator). - to receive(:new).with(github_repo, an_instance_of(Group), user, access_params). - and_return(double(execute: true)) + context "when current user can't create namespaces" do + before do + user.update_attribute(:can_create_group, false) + end - post :create, format: :js + it "doesn't create the namespace" do + expect(Gitlab::GithubImport::ProjectCreator). + to receive(:new).and_return(double(execute: true)) + + expect { post :create, format: :js }.not_to change(Namespace, :count) + end + + it "takes the current user's namespace" do + expect(Gitlab::GithubImport::ProjectCreator). + to receive(:new).with(github_repo, user.namespace, user, access_params). + and_return(double(execute: true)) + + post :create, format: :js + end end end end diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb index e8cf6aa7767..6f75ebb16c8 100644 --- a/spec/controllers/import/gitlab_controller_spec.rb +++ b/spec/controllers/import/gitlab_controller_spec.rb @@ -136,21 +136,42 @@ describe Import::GitlabController do end context "when a namespace with the GitLab.com user's username doesn't exist" do - it "creates the namespace" do - expect(Gitlab::GitlabImport::ProjectCreator). - to receive(:new).and_return(double(execute: true)) + context "when current user can create namespaces" do + it "creates the namespace" do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).and_return(double(execute: true)) - post :create, format: :js + expect { post :create, format: :js }.to change(Namespace, :count).by(1) + end + + it "takes the new namespace" do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, an_instance_of(Group), user, access_params). + and_return(double(execute: true)) - expect(Namespace.where(name: other_username).first).not_to be_nil + post :create, format: :js + end end - it "takes the new namespace" do - expect(Gitlab::GitlabImport::ProjectCreator). - to receive(:new).with(gitlab_repo, an_instance_of(Group), user, access_params). - and_return(double(execute: true)) + context "when current user can't create namespaces" do + before do + user.update_attribute(:can_create_group, false) + end - post :create, format: :js + it "doesn't create the namespace" do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).and_return(double(execute: true)) + + expect { post :create, format: :js }.not_to change(Namespace, :count) + end + + it "takes the current user's namespace" do + expect(Gitlab::GitlabImport::ProjectCreator). + to receive(:new).with(gitlab_repo, user.namespace, user, access_params). + and_return(double(execute: true)) + + post :create, format: :js + end end end end diff --git a/spec/helpers/import_helper_spec.rb b/spec/helpers/import_helper_spec.rb index 3391234e9f5..187b891b927 100644 --- a/spec/helpers/import_helper_spec.rb +++ b/spec/helpers/import_helper_spec.rb @@ -1,6 +1,30 @@ require 'rails_helper' describe ImportHelper do + describe '#import_project_target' do + let(:user) { create(:user) } + + before do + allow(helper).to receive(:current_user).and_return(user) + end + + context 'when current user can create namespaces' do + it 'returns project namespace' do + user.update_attribute(:can_create_group, true) + + expect(helper.import_project_target('asd', 'vim')).to eq 'asd/vim' + end + end + + context 'when current user can not create namespaces' do + it "takes the current user's namespace" do + user.update_attribute(:can_create_group, false) + + expect(helper.import_project_target('asd', 'vim')).to eq "#{user.namespace_path}/vim" + end + end + end + describe '#github_project_link' do context 'when provider does not specify a custom URL' do it 'uses default GitHub URL' do diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index aff022a573e..5dfb33f4b28 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -18,12 +18,12 @@ describe Issues::CloseService, services: true do context "valid params" do before do perform_enqueued_jobs do - @issue = described_class.new(project, user, {}).execute(issue) + described_class.new(project, user).execute(issue) end end - it { expect(@issue).to be_valid } - it { expect(@issue).to be_closed } + it { expect(issue).to be_valid } + it { expect(issue).to be_closed } it 'sends email to user2 about assign of new issue' do email = ActionMailer::Base.deliveries.last @@ -32,7 +32,7 @@ describe Issues::CloseService, services: true do end it 'creates system note about issue reassign' do - note = @issue.notes.last + note = issue.notes.last expect(note.note).to include "Status changed to closed" end @@ -44,23 +44,43 @@ describe Issues::CloseService, services: true do context 'current user is not authorized to close issue' do before do perform_enqueued_jobs do - @issue = described_class.new(project, guest).execute(issue) + described_class.new(project, guest).execute(issue) end end it 'does not close the issue' do - expect(@issue).to be_open + expect(issue).to be_open end end - context "external issue tracker" do + context 'when issue is not confidential' do + it 'executes issue hooks' do + expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) + expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) + + described_class.new(project, user).execute(issue) + end + end + + context 'when issue is confidential' do + it 'executes confidential issue hooks' do + issue = create(:issue, :confidential, project: project) + + expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) + expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) + + described_class.new(project, user).execute(issue) + end + end + + context 'external issue tracker' do before do allow(project).to receive(:default_issues_tracker?).and_return(false) - @issue = described_class.new(project, user, {}).execute(issue) + described_class.new(project, user).execute(issue) end - it { expect(@issue).to be_valid } - it { expect(@issue).to be_opened } + it { expect(issue).to be_valid } + it { expect(issue).to be_opened } it { expect(todo.reload).to be_pending } end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index fcc3c0a00bd..58569ba96c3 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -72,6 +72,24 @@ describe Issues::CreateService, services: true do expect(issue.milestone).not_to eq milestone end end + + it 'executes issue hooks when issue is not confidential' do + opts = { title: 'Title', description: 'Description', confidential: false } + + expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) + expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) + + described_class.new(project, user, opts).execute + end + + it 'executes confidential issue hooks when issue is confidential' do + opts = { title: 'Title', description: 'Description', confidential: true } + + expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) + expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) + + described_class.new(project, user, opts).execute + end end it_behaves_like 'new issuable record that supports slash commands' diff --git a/spec/services/issues/reopen_service_spec.rb b/spec/services/issues/reopen_service_spec.rb index 34a89fcd4e1..93a8270fd16 100644 --- a/spec/services/issues/reopen_service_spec.rb +++ b/spec/services/issues/reopen_service_spec.rb @@ -1,24 +1,50 @@ require 'spec_helper' describe Issues::ReopenService, services: true do - let(:guest) { create(:user) } - let(:issue) { create(:issue, :closed) } - let(:project) { issue.project } - - before do - project.team << [guest, :guest] - end + let(:project) { create(:empty_project) } + let(:issue) { create(:issue, :closed, project: project) } describe '#execute' do - context 'current user is not authorized to reopen issue' do + context 'when user is not authorized to reopen issue' do before do + guest = create(:user) + project.team << [guest, :guest] + perform_enqueued_jobs do - @issue = described_class.new(project, guest).execute(issue) + described_class.new(project, guest).execute(issue) end end it 'does not reopen the issue' do - expect(@issue).to be_closed + expect(issue).to be_closed + end + end + + context 'when user is authrized to reopen issue' do + let(:user) { create(:user) } + + before do + project.team << [user, :master] + end + + context 'when issue is not confidential' do + it 'executes issue hooks' do + expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) + expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) + + described_class.new(project, user).execute(issue) + end + end + + context 'when issue is confidential' do + it 'executes confidential issue hooks' do + issue = create(:issue, :confidential, :closed, project: project) + + expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) + expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) + + described_class.new(project, user).execute(issue) + end end end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 0313f424463..4f5375a3583 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -23,11 +23,15 @@ describe Issues::UpdateService, services: true do describe 'execute' do def find_note(starting_with) - @issue.notes.find do |note| + issue.notes.find do |note| note && note.note.start_with?(starting_with) end end + def update_issue(opts) + described_class.new(project, user, opts).execute(issue) + end + context "valid params" do before do opts = { @@ -35,23 +39,20 @@ describe Issues::UpdateService, services: true do description: 'Also please fix', assignee_id: user2.id, state_event: 'close', - label_ids: [label.id], - confidential: true + label_ids: [label.id] } perform_enqueued_jobs do - @issue = Issues::UpdateService.new(project, user, opts).execute(issue) + update_issue(opts) end - - @issue.reload end - it { expect(@issue).to be_valid } - it { expect(@issue.title).to eq('New title') } - it { expect(@issue.assignee).to eq(user2) } - it { expect(@issue).to be_closed } - it { expect(@issue.labels.count).to eq(1) } - it { expect(@issue.labels.first.title).to eq(label.name) } + it { expect(issue).to be_valid } + it { expect(issue.title).to eq('New title') } + it { expect(issue.assignee).to eq(user2) } + it { expect(issue).to be_closed } + it { expect(issue.labels.count).to eq(1) } + it { expect(issue.labels.first.title).to eq(label.name) } it 'sends email to user2 about assign of new issue and email to user3 about issue unassignment' do deliveries = ActionMailer::Base.deliveries @@ -81,18 +82,35 @@ describe Issues::UpdateService, services: true do expect(note).not_to be_nil expect(note.note).to eq 'Changed title: **{-Old-} title** → **{+New+} title**' end + end + + context 'when issue turns confidential' do + let(:opts) do + { + title: 'New title', + description: 'Also please fix', + assignee_id: user2.id, + state_event: 'close', + label_ids: [label.id], + confidential: true + } + end it 'creates system note about confidentiality change' do + update_issue(confidential: true) + note = find_note('Made the issue confidential') expect(note).not_to be_nil expect(note.note).to eq 'Made the issue confidential' end - end - def update_issue(opts) - @issue = Issues::UpdateService.new(project, user, opts).execute(issue) - @issue.reload + it 'executes confidential issue hooks' do + expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) + expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) + + update_issue(confidential: true) + end end context 'todos' do @@ -100,7 +118,7 @@ describe Issues::UpdateService, services: true do context 'when the title change' do before do - update_issue({ title: 'New title' }) + update_issue(title: 'New title') end it 'marks pending todos as done' do @@ -110,7 +128,7 @@ describe Issues::UpdateService, services: true do context 'when the description change' do before do - update_issue({ description: 'Also please fix' }) + update_issue(description: 'Also please fix') end it 'marks todos as done' do @@ -120,7 +138,7 @@ describe Issues::UpdateService, services: true do context 'when is reassigned' do before do - update_issue({ assignee: user2 }) + update_issue(assignee: user2) end it 'marks previous assignee todos as done' do @@ -144,7 +162,7 @@ describe Issues::UpdateService, services: true do context 'when the milestone change' do before do - update_issue({ milestone: create(:milestone) }) + update_issue(milestone: create(:milestone)) end it 'marks todos as done' do @@ -154,7 +172,7 @@ describe Issues::UpdateService, services: true do context 'when the labels change' do before do - update_issue({ label_ids: [label.id] }) + update_issue(label_ids: [label.id]) end it 'marks todos as done' do @@ -165,6 +183,7 @@ describe Issues::UpdateService, services: true do context 'when the issue is relabeled' do let!(:non_subscriber) { create(:user) } + let!(:subscriber) do create(:user).tap do |u| label.toggle_subscription(u) @@ -176,7 +195,7 @@ describe Issues::UpdateService, services: true do opts = { label_ids: [label.id] } perform_enqueued_jobs do - @issue = Issues::UpdateService.new(project, user, opts).execute(issue) + @issue = described_class.new(project, user, opts).execute(issue) end should_email(subscriber) @@ -190,7 +209,7 @@ describe Issues::UpdateService, services: true do opts = { label_ids: [label.id, label2.id] } perform_enqueued_jobs do - @issue = Issues::UpdateService.new(project, user, opts).execute(issue) + @issue = described_class.new(project, user, opts).execute(issue) end should_not_email(subscriber) @@ -201,7 +220,7 @@ describe Issues::UpdateService, services: true do opts = { label_ids: [label2.id] } perform_enqueued_jobs do - @issue = Issues::UpdateService.new(project, user, opts).execute(issue) + @issue = described_class.new(project, user, opts).execute(issue) end should_not_email(subscriber) @@ -210,13 +229,15 @@ describe Issues::UpdateService, services: true do end end - context 'when Issue has tasks' do - before { update_issue({ description: "- [ ] Task 1\n- [ ] Task 2" }) } + context 'when issue has tasks' do + before do + update_issue(description: "- [ ] Task 1\n- [ ] Task 2") + end - it { expect(@issue.tasks?).to eq(true) } + it { expect(issue.tasks?).to eq(true) } context 'when tasks are marked as completed' do - before { update_issue({ description: "- [x] Task 1\n- [X] Task 2" }) } + before { update_issue(description: "- [x] Task 1\n- [X] Task 2") } it 'creates system note about task status change' do note1 = find_note('Marked the task **Task 1** as completed') @@ -229,8 +250,8 @@ describe Issues::UpdateService, services: true do context 'when tasks are marked as incomplete' do before do - update_issue({ description: "- [x] Task 1\n- [X] Task 2" }) - update_issue({ description: "- [ ] Task 1\n- [ ] Task 2" }) + update_issue(description: "- [x] Task 1\n- [X] Task 2") + update_issue(description: "- [ ] Task 1\n- [ ] Task 2") end it 'creates system note about task status change' do @@ -244,8 +265,8 @@ describe Issues::UpdateService, services: true do context 'when tasks position has been modified' do before do - update_issue({ description: "- [x] Task 1\n- [X] Task 2" }) - update_issue({ description: "- [x] Task 1\n- [ ] Task 3\n- [ ] Task 2" }) + update_issue(description: "- [x] Task 1\n- [X] Task 2") + update_issue(description: "- [x] Task 1\n- [ ] Task 3\n- [ ] Task 2") end it 'does not create a system note' do @@ -257,8 +278,8 @@ describe Issues::UpdateService, services: true do context 'when a Task list with a completed item is totally replaced' do before do - update_issue({ description: "- [ ] Task 1\n- [X] Task 2" }) - update_issue({ description: "- [ ] One\n- [ ] Two\n- [ ] Three" }) + update_issue(description: "- [ ] Task 1\n- [X] Task 2") + update_issue(description: "- [ ] One\n- [ ] Two\n- [ ] Three") end it 'does not create a system note referencing the position the old item' do @@ -269,7 +290,7 @@ describe Issues::UpdateService, services: true do it 'does not generate a new note at all' do expect do - update_issue({ description: "- [ ] One\n- [ ] Two\n- [ ] Three" }) + update_issue(description: "- [ ] One\n- [ ] Two\n- [ ] Three") end.not_to change { Note.count } end end @@ -277,7 +298,7 @@ describe Issues::UpdateService, services: true do context 'updating labels' do let(:label3) { create(:label, project: project) } - let(:result) { Issues::UpdateService.new(project, user, params).execute(issue).reload } + let(:result) { described_class.new(project, user, params).execute(issue).reload } context 'when add_label_ids and label_ids are passed' do let(:params) { { label_ids: [label.id], add_label_ids: [label3.id] } } |