From ace19ec58f39faa084e6f5372732e3c094011c8e Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Fri, 5 Aug 2016 12:44:13 -0500 Subject: [PATCH 01/19] Style deploy button --- app/assets/stylesheets/pages/environments.scss | 17 +++++++++++++++++ app/views/projects/deployments/_actions.haml | 6 +++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index e160d676e35..e9097d41c61 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -1,5 +1,22 @@ .environments { + .commit-title { margin: 0; } + + .fa-play { + font-size: 14px; + } + + .dropdown-new { + color: $table-text-gray; + } + + .dropdown-menu { + + .fa { + margin-right: 6px; + color: $table-text-gray; + } + } } diff --git a/app/views/projects/deployments/_actions.haml b/app/views/projects/deployments/_actions.haml index f70dba224fa..f7bf3b834ef 100644 --- a/app/views/projects/deployments/_actions.haml +++ b/app/views/projects/deployments/_actions.haml @@ -2,9 +2,9 @@ .pull-right - actions = deployment.manual_actions - if actions.present? - .btn-group.inline - .btn-group - %a.dropdown-toggle.btn.btn-default{type: 'button', 'data-toggle' => 'dropdown'} + .inline + .dropdown + %a.dropdown-new.btn.btn-default{type: 'button', 'data-toggle' => 'dropdown'} = icon("play") %b.caret %ul.dropdown-menu.dropdown-menu-align-right From 415921a7f38a19bc907ce0b3db21e4e66e9b1ac2 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Fri, 5 Aug 2016 14:08:39 -0500 Subject: [PATCH 02/19] Add avatar to commit message; environment style updates to match pipelines page --- app/assets/stylesheets/pages/environments.scss | 13 +++++++++++++ app/helpers/avatars_helper.rb | 2 -- app/views/projects/deployments/_commit.html.haml | 8 ++++++-- .../projects/environments/_environment.html.haml | 7 ++++--- app/views/projects/environments/index.html.haml | 4 ++-- 5 files changed, 25 insertions(+), 9 deletions(-) diff --git a/app/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss index e9097d41c61..55f9d4a0011 100644 --- a/app/assets/stylesheets/pages/environments.scss +++ b/app/assets/stylesheets/pages/environments.scss @@ -19,4 +19,17 @@ color: $table-text-gray; } } + + .branch-name { + color: $gl-dark-link-color; + } +} + +.table.builds.environments { + min-width: 500px; + + .icon-container { + width: 20px; + text-align: center; + } } diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index 6ff40c6b461..010246b6c23 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -8,8 +8,6 @@ module AvatarsHelper })) end - private - def user_avatar(options = {}) avatar_size = options[:size] || 16 user_name = options[:user].try(:name) || options[:user_name] diff --git a/app/views/projects/deployments/_commit.html.haml b/app/views/projects/deployments/_commit.html.haml index 0f9d9512d88..a90c8733862 100644 --- a/app/views/projects/deployments/_commit.html.haml +++ b/app/views/projects/deployments/_commit.html.haml @@ -1,12 +1,16 @@ %div.branch-commit - if deployment.ref - = link_to deployment.ref, namespace_project_commits_path(@project.namespace, @project, deployment.ref), class: "monospace" - · + .icon-container + = deployment.tag? ? icon('tag') : icon('code-fork') + = link_to deployment.ref, namespace_project_commits_path(@project.namespace, @project, deployment.ref), class: "monospace branch-name" + .icon-container + = custom_icon("icon_commit") = link_to deployment.short_sha, namespace_project_commit_path(@project.namespace, @project, deployment.sha), class: "commit-id monospace" %p.commit-title %span - if commit_title = deployment.commit_title + = user_avatar(user: deployment.user, size: 20) = link_to_gfm commit_title, namespace_project_commit_path(@project.namespace, @project, deployment.sha), class: "commit-row-message" - else Cant find HEAD commit for this branch diff --git a/app/views/projects/environments/_environment.html.haml b/app/views/projects/environments/_environment.html.haml index e2453395602..d04967fa72f 100644 --- a/app/views/projects/environments/_environment.html.haml +++ b/app/views/projects/environments/_environment.html.haml @@ -2,8 +2,7 @@ %tr.environment %td - %strong - = link_to environment.name, namespace_project_environment_path(@project.namespace, @project, environment) + = link_to environment.name, namespace_project_environment_path(@project.namespace, @project, environment) %td - if last_deployment @@ -14,7 +13,9 @@ %td - if last_deployment - #{time_ago_with_tooltip(last_deployment.created_at)} + %p.finished-at + = icon("calendar") + #{time_ago_with_tooltip(last_deployment.created_at, short_format: true, skip_js: true)} %td = render 'projects/deployments/actions', deployment: last_deployment diff --git a/app/views/projects/environments/index.html.haml b/app/views/projects/environments/index.html.haml index a6dd34653ab..fe8ddb4716e 100644 --- a/app/views/projects/environments/index.html.haml +++ b/app/views/projects/environments/index.html.haml @@ -23,10 +23,10 @@ New environment - else .table-holder - %table.table.environments + %table.table.builds.environments %tbody %th Environment %th Last deployment - %th Date + %th %th = render @environments From 60a347228d90ce68eedb5bf4b75a4cad94a1dfb2 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Fri, 5 Aug 2016 14:54:17 -0500 Subject: [PATCH 03/19] Format environment history page --- app/views/projects/environments/show.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/projects/environments/show.html.haml b/app/views/projects/environments/show.html.haml index a07436ad7c9..8f8c1c4ce22 100644 --- a/app/views/projects/environments/show.html.haml +++ b/app/views/projects/environments/show.html.haml @@ -23,13 +23,13 @@ = link_to "Read more", help_page_path("ci/environments"), class: "btn btn-success" - else .table-holder - %table.table.environments + %table.table.builds.environments %thead %tr %th ID %th Commit %th Build - %th Date + %th %th = render @deployments From 0a15ad31ad4bafe0a87cb0c4fd345a5f82c44799 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Mon, 8 Aug 2016 11:39:24 -0500 Subject: [PATCH 04/19] Add deployment ID and gravatar to environments page --- app/views/projects/deployments/_commit.html.haml | 2 +- app/views/projects/environments/_environment.html.haml | 9 ++++++--- app/views/projects/environments/index.html.haml | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/views/projects/deployments/_commit.html.haml b/app/views/projects/deployments/_commit.html.haml index a90c8733862..28813babd7b 100644 --- a/app/views/projects/deployments/_commit.html.haml +++ b/app/views/projects/deployments/_commit.html.haml @@ -10,7 +10,7 @@ %p.commit-title %span - if commit_title = deployment.commit_title - = user_avatar(user: deployment.user, size: 20) + = author_avatar(deployment.commit, size: 20) = link_to_gfm commit_title, namespace_project_commit_path(@project.namespace, @project, deployment.sha), class: "commit-row-message" - else Cant find HEAD commit for this branch diff --git a/app/views/projects/environments/_environment.html.haml b/app/views/projects/environments/_environment.html.haml index d04967fa72f..718eca15741 100644 --- a/app/views/projects/environments/_environment.html.haml +++ b/app/views/projects/environments/_environment.html.haml @@ -4,6 +4,11 @@ %td = link_to environment.name, namespace_project_environment_path(@project.namespace, @project, environment) + %td + - if last_deployment + = user_avatar(user: last_deployment.user, size: 20) + %span ##{last_deployment.id} + %td - if last_deployment = render 'projects/deployments/commit', deployment: last_deployment @@ -13,9 +18,7 @@ %td - if last_deployment - %p.finished-at - = icon("calendar") - #{time_ago_with_tooltip(last_deployment.created_at, short_format: true, skip_js: true)} + #{time_ago_with_tooltip(last_deployment.created_at)} %td = render 'projects/deployments/actions', deployment: last_deployment diff --git a/app/views/projects/environments/index.html.haml b/app/views/projects/environments/index.html.haml index fe8ddb4716e..8bb238bae4e 100644 --- a/app/views/projects/environments/index.html.haml +++ b/app/views/projects/environments/index.html.haml @@ -26,6 +26,7 @@ %table.table.builds.environments %tbody %th Environment + %th Deployment ID %th Last deployment %th %th From f6c0c96b80b971ffaa7aa49553e13f6849f8ec4a Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Mon, 8 Aug 2016 11:43:12 -0500 Subject: [PATCH 05/19] Add gravatars to build history --- app/views/projects/deployments/_deployment.html.haml | 1 + app/views/projects/environments/_environment.html.haml | 2 +- app/views/projects/environments/index.html.haml | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/views/projects/deployments/_deployment.html.haml b/app/views/projects/deployments/_deployment.html.haml index baf02f1e6a0..cd95841ca5a 100644 --- a/app/views/projects/deployments/_deployment.html.haml +++ b/app/views/projects/deployments/_deployment.html.haml @@ -8,6 +8,7 @@ %td - if deployment.deployable = link_to [@project.namespace.becomes(Namespace), @project, deployment.deployable] do + = user_avatar(user: deployment.user, size: 20) = "#{deployment.deployable.name} (##{deployment.deployable.id})" %td diff --git a/app/views/projects/environments/_environment.html.haml b/app/views/projects/environments/_environment.html.haml index 718eca15741..36a6162a5a8 100644 --- a/app/views/projects/environments/_environment.html.haml +++ b/app/views/projects/environments/_environment.html.haml @@ -7,7 +7,7 @@ %td - if last_deployment = user_avatar(user: last_deployment.user, size: 20) - %span ##{last_deployment.id} + %strong ##{last_deployment.id} %td - if last_deployment diff --git a/app/views/projects/environments/index.html.haml b/app/views/projects/environments/index.html.haml index 8bb238bae4e..b3eb5b0011a 100644 --- a/app/views/projects/environments/index.html.haml +++ b/app/views/projects/environments/index.html.haml @@ -26,8 +26,8 @@ %table.table.builds.environments %tbody %th Environment - %th Deployment ID - %th Last deployment + %th Last Deployment + %th Commit %th %th = render @environments From 7d65d2ec2b808a796efdada83cd0ec3613ca693b Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 11 Aug 2016 14:31:19 +0200 Subject: [PATCH 06/19] Corrected links/usernames in performance guide This fixes two broken links in the performance guide and removes the mention of Josh as he no longer works for GitLab. [ci skip] --- doc/development/performance.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/doc/development/performance.md b/doc/development/performance.md index fb37b3a889c..7ff603e2c4a 100644 --- a/doc/development/performance.md +++ b/doc/development/performance.md @@ -15,8 +15,8 @@ The process of solving performance problems is roughly as follows: 3. Add your findings based on the measurement period (screenshots of graphs, timings, etc) to the issue mentioned in step 1. 4. Solve the problem. -5. Create a merge request, assign the "performance" label and ping the right - people (e.g. [@yorickpeterse][yorickpeterse] and [@joshfng][joshfng]). +5. Create a merge request, assign the "Performance" label and assign it to + [@yorickpeterse][yorickpeterse] for reviewing. 6. Once a change has been deployed make sure to _again_ measure for at least 24 hours to see if your changes have any impact on the production environment. 7. Repeat until you're done. @@ -36,8 +36,8 @@ graphs/dashboards. GitLab provides two built-in tools to aid the process of improving performance: -* [Sherlock](doc/development/profiling.md#sherlock) -* [GitLab Performance Monitoring](doc/monitoring/performance/monitoring.md) +* [Sherlock](profiling.md#sherlock) +* [GitLab Performance Monitoring](../monitoring/performance/monitoring.md) GitLab employees can use GitLab.com's performance monitoring systems located at , this requires you to log in using your @@ -254,5 +254,4 @@ referencing an object directly may even slow code down. [#15607]: https://gitlab.com/gitlab-org/gitlab-ce/issues/15607 [yorickpeterse]: https://gitlab.com/u/yorickpeterse -[joshfng]: https://gitlab.com/u/joshfng [anti-pattern]: https://en.wikipedia.org/wiki/Anti-pattern From c2a1011f529c21b8b571edc0daaf1f4875509e48 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 11 Aug 2016 08:45:14 -0700 Subject: [PATCH 07/19] Remove unused SpamReport model; this was renamed to SpamLog --- app/models/spam_report.rb | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 app/models/spam_report.rb diff --git a/app/models/spam_report.rb b/app/models/spam_report.rb deleted file mode 100644 index cdc7321b08e..00000000000 --- a/app/models/spam_report.rb +++ /dev/null @@ -1,5 +0,0 @@ -class SpamReport < ActiveRecord::Base - belongs_to :user - - validates :user, presence: true -end From cb8a425ba42e9be23b8712ed29b1db2dcc6bd139 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 28 May 2016 19:54:17 -0700 Subject: [PATCH 08/19] Fix bug where destroying a namespace would not always destroy projects There is a race condition in DestroyGroupService now that projects are deleted asynchronously: 1. User attempts to delete group 2. DestroyGroupService iterates through all projects and schedules a Sidekiq job to delete each Project 3. DestroyGroupService destroys the Group, leaving all its projects without a namespace 4. Projects::DestroyService runs later but the can?(current_user, :remove_project) is `false` because the user no longer has permission to destroy projects with no namespace. 5. This leaves the project in pending_delete state with no namespace/group. Projects without a namespace or group also adds another problem: it's not possible to destroy the container registry tags, since container_registry_path_with_namespace is the wrong value. The fix is to destroy the group asynchronously and to run execute directly on Projects::DestroyService. Closes #17893 --- CHANGELOG | 1 + app/controllers/admin/groups_controller.rb | 4 +- app/controllers/groups_controller.rb | 4 +- app/models/namespace.rb | 2 + app/services/delete_user_service.rb | 7 ++- app/services/destroy_group_service.rb | 16 ++++- app/workers/group_destroy_worker.rb | 17 ++++++ db/migrate/20140407135544_fix_namespaces.rb | 10 +++- ...0805041956_add_deleted_at_to_namespaces.rb | 12 ++++ db/schema.rb | 2 + .../admin/groups_controller_spec.rb | 24 ++++++++ spec/controllers/groups_controller_spec.rb | 29 +++++++++ spec/requests/api/users_spec.rb | 2 + spec/services/delete_user_service_spec.rb | 6 +- spec/services/destroy_group_service_spec.rb | 60 ++++++++++++------- spec/workers/group_destroy_worker_spec.rb | 19 ++++++ 16 files changed, 180 insertions(+), 35 deletions(-) create mode 100644 app/workers/group_destroy_worker.rb create mode 100644 db/migrate/20160805041956_add_deleted_at_to_namespaces.rb create mode 100644 spec/controllers/admin/groups_controller_spec.rb create mode 100644 spec/workers/group_destroy_worker_spec.rb diff --git a/CHANGELOG b/CHANGELOG index 28834c1129a..85270bc6001 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -92,6 +92,7 @@ v 8.11.0 (unreleased) - Bump gitlab_git to lazy load compare commits - Reduce number of queries made for merge_requests/:id/diffs - Sensible state specific default sort order for issues and merge requests !5453 (tomb0y) + - Fix bug where destroying a namespace would not always destroy projects - Fix RequestProfiler::Middleware error when code is reloaded in development - Catch what warden might throw when profiling requests to re-throw it - Avoid commit lookup on diff_helper passing existing local variable to the helper method diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index f3a88a8e6c8..4ce18321649 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -48,9 +48,9 @@ class Admin::GroupsController < Admin::ApplicationController end def destroy - DestroyGroupService.new(@group, current_user).execute + DestroyGroupService.new(@group, current_user).async_execute - redirect_to admin_groups_path, notice: 'Group was successfully deleted.' + redirect_to admin_groups_path, alert: "Group '#{@group.name}' was scheduled for deletion." end private diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 6780a6d4d87..cb82d62616c 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -87,9 +87,9 @@ class GroupsController < Groups::ApplicationController end def destroy - DestroyGroupService.new(@group, current_user).execute + DestroyGroupService.new(@group, current_user).async_execute - redirect_to root_path, alert: "Group '#{@group.name}' was successfully deleted." + redirect_to root_path, alert: "Group '#{@group.name}' was scheduled for deletion." end protected diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 8b52cc824cd..7c29d27ce97 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -1,4 +1,6 @@ class Namespace < ActiveRecord::Base + acts_as_paranoid + include Sortable include Gitlab::ShellAdapter diff --git a/app/services/delete_user_service.rb b/app/services/delete_user_service.rb index 2f237de813c..eaff88d6463 100644 --- a/app/services/delete_user_service.rb +++ b/app/services/delete_user_service.rb @@ -21,6 +21,11 @@ class DeleteUserService ::Projects::DestroyService.new(project, current_user, skip_repo: true).async_execute end - user.destroy + # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing + namespace = user.namespace + user_data = user.destroy + namespace.really_destroy! + + user_data end end diff --git a/app/services/destroy_group_service.rb b/app/services/destroy_group_service.rb index a4ebccb5606..0081364b8aa 100644 --- a/app/services/destroy_group_service.rb +++ b/app/services/destroy_group_service.rb @@ -5,13 +5,23 @@ class DestroyGroupService @group, @current_user = group, user end + def async_execute + group.transaction do + # Soft delete via paranoia gem + group.destroy + job_id = GroupDestroyWorker.perform_async(group.id, current_user.id) + Rails.logger.info("User #{current_user.id} scheduled a deletion of group ID #{group.id} with job ID #{job_id}") + end + end + def execute group.projects.each do |project| + # Execute the destruction of the models immediately to ensure atomic cleanup. # Skip repository removal because we remove directory with namespace - # that contain all this repositories - ::Projects::DestroyService.new(project, current_user, skip_repo: true).async_execute + # that contain all these repositories + ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute end - group.destroy + group.really_destroy! end end diff --git a/app/workers/group_destroy_worker.rb b/app/workers/group_destroy_worker.rb new file mode 100644 index 00000000000..5048746f09b --- /dev/null +++ b/app/workers/group_destroy_worker.rb @@ -0,0 +1,17 @@ +class GroupDestroyWorker + include Sidekiq::Worker + + sidekiq_options queue: :default + + def perform(group_id, user_id) + begin + group = Group.with_deleted.find(group_id) + rescue ActiveRecord::RecordNotFound + return + end + + user = User.find(user_id) + + DestroyGroupService.new(group, user).execute + end +end diff --git a/db/migrate/20140407135544_fix_namespaces.rb b/db/migrate/20140407135544_fix_namespaces.rb index 91374966698..0026ce645a6 100644 --- a/db/migrate/20140407135544_fix_namespaces.rb +++ b/db/migrate/20140407135544_fix_namespaces.rb @@ -1,8 +1,14 @@ # rubocop:disable all class FixNamespaces < ActiveRecord::Migration + DOWNTIME = false + def up - Namespace.where('name <> path and type is null').each do |namespace| - namespace.update_attribute(:name, namespace.path) + namespaces = exec_query('SELECT id, path FROM namespaces WHERE name <> path and type is null') + + namespaces.each do |row| + id = row['id'] + path = row['path'] + exec_query("UPDATE namespaces SET name = '#{path}' WHERE id = #{id}") end end diff --git a/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb b/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb new file mode 100644 index 00000000000..a853de3abfb --- /dev/null +++ b/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb @@ -0,0 +1,12 @@ +class AddDeletedAtToNamespaces < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def change + add_column :namespaces, :deleted_at, :datetime + add_concurrent_index :namespaces, :deleted_at + end +end diff --git a/db/schema.rb b/db/schema.rb index 6c85e1e9dba..1de2cdcf026 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -640,9 +640,11 @@ ActiveRecord::Schema.define(version: 20160810142633) do t.boolean "share_with_group_lock", default: false t.integer "visibility_level", default: 20, null: false t.boolean "request_access_enabled", default: true, null: false + t.datetime "deleted_at" end add_index "namespaces", ["created_at"], name: "index_namespaces_on_created_at", using: :btree + add_index "namespaces", ["deleted_at"], name: "index_namespaces_on_deleted_at", using: :btree add_index "namespaces", ["name"], name: "index_namespaces_on_name", unique: true, using: :btree add_index "namespaces", ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"} add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb new file mode 100644 index 00000000000..0239aea47fb --- /dev/null +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe Admin::GroupsController do + let(:group) { create(:group) } + let(:project) { create(:project, namespace: group) } + let(:admin) { create(:admin) } + + before do + sign_in(admin) + Sidekiq::Testing.fake! + end + + describe 'DELETE #destroy' do + it 'schedules a group destroy' do + expect { delete :destroy, id: project.group.path }.to change(GroupDestroyWorker.jobs, :size).by(1) + end + + it 'redirects to the admin group path' do + delete :destroy, id: project.group.path + + expect(response).to redirect_to(admin_groups_path) + end + end +end diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index cd98fecd0c7..4ae6364207b 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -75,4 +75,33 @@ describe GroupsController do end end end + + describe 'DELETE #destroy' do + context 'as another user' do + it 'returns 404' do + sign_in(create(:user)) + + delete :destroy, id: group.path + + expect(response.status).to eq(404) + end + end + + context 'as the group owner' do + before do + Sidekiq::Testing.fake! + sign_in(user) + end + + it 'schedules a group destroy' do + expect { delete :destroy, id: group.path }.to change(GroupDestroyWorker.jobs, :size).by(1) + end + + it 'redirects to the root path' do + delete :destroy, id: group.path + + expect(response).to redirect_to(root_path) + end + end + end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index e0e041b4e15..0bbba64a6d5 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -564,12 +564,14 @@ describe API::API, api: true do end describe "DELETE /users/:id" do + let!(:namespace) { user.namespace } before { admin } it "deletes user" do delete api("/users/#{user.id}", admin) expect(response).to have_http_status(200) expect { User.find(user.id) }.to raise_error ActiveRecord::RecordNotFound + expect { Namespace.find(namespace.id) }.to raise_error ActiveRecord::RecordNotFound expect(json_response['email']).to eq(user.email) end diff --git a/spec/services/delete_user_service_spec.rb b/spec/services/delete_user_service_spec.rb index 630458f9efc..418a12a83a9 100644 --- a/spec/services/delete_user_service_spec.rb +++ b/spec/services/delete_user_service_spec.rb @@ -9,9 +9,11 @@ describe DeleteUserService, services: true do context 'no options are given' do it 'deletes the user' do - DeleteUserService.new(current_user).execute(user) + user_data = DeleteUserService.new(current_user).execute(user) - expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { user_data['email'].to eq(user.email) } + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Namespace.with_deleted.find(user.namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) end it 'will delete the project in the near future' do diff --git a/spec/services/destroy_group_service_spec.rb b/spec/services/destroy_group_service_spec.rb index eca8ddd8ea4..da724643604 100644 --- a/spec/services/destroy_group_service_spec.rb +++ b/spec/services/destroy_group_service_spec.rb @@ -7,38 +7,52 @@ describe DestroyGroupService, services: true do let!(:gitlab_shell) { Gitlab::Shell.new } let!(:remove_path) { group.path + "+#{group.id}+deleted" } - context 'database records' do - before do - destroy_group(group, user) - end - - it { expect(Group.all).not_to include(group) } - it { expect(Project.all).not_to include(project) } - end - - context 'file system' do - context 'Sidekiq inline' do + shared_examples 'group destruction' do |async| + context 'database records' do before do - # Run sidekiq immediatly to check that renamed dir will be removed - Sidekiq::Testing.inline! { destroy_group(group, user) } + destroy_group(group, user, async) end - it { expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_falsey } - it { expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey } + it { expect(Group.all).not_to include(group) } + it { expect(Project.all).not_to include(project) } end - context 'Sidekiq fake' do - before do - # Dont run sidekiq to check if renamed repository exists - Sidekiq::Testing.fake! { destroy_group(group, user) } + context 'file system' do + context 'Sidekiq inline' do + before do + # Run sidekiq immediatly to check that renamed dir will be removed + Sidekiq::Testing.inline! { destroy_group(group, user, async) } + end + + it { expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_falsey } + it { expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_falsey } end - it { expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_falsey } - it { expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_truthy } + context 'Sidekiq fake' do + before do + # Dont run sidekiq to check if renamed repository exists + Sidekiq::Testing.fake! { destroy_group(group, user, async) } + end + + it { expect(gitlab_shell.exists?(project.repository_storage_path, group.path)).to be_falsey } + it { expect(gitlab_shell.exists?(project.repository_storage_path, remove_path)).to be_truthy } + end + end + + def destroy_group(group, user, async) + if async + DestroyGroupService.new(group, user).async_execute + else + DestroyGroupService.new(group, user).execute + end end end - def destroy_group(group, user) - DestroyGroupService.new(group, user).execute + describe 'asynchronous delete' do + it_behaves_like 'group destruction', true + end + + describe 'synchronous delete' do + it_behaves_like 'group destruction', false end end diff --git a/spec/workers/group_destroy_worker_spec.rb b/spec/workers/group_destroy_worker_spec.rb new file mode 100644 index 00000000000..4e4eaf9b2f7 --- /dev/null +++ b/spec/workers/group_destroy_worker_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +describe GroupDestroyWorker do + let(:group) { create(:group) } + let(:user) { create(:admin) } + let!(:project) { create(:project, namespace: group) } + + subject { GroupDestroyWorker.new } + + describe "#perform" do + it "deletes the project" do + subject.perform(group.id, user.id) + + expect(Group.all).not_to include(group) + expect(Project.all).not_to include(project) + expect(Dir.exist?(project.path)).to be_falsey + end + end +end From d7b681512bb738b9b2ca0c56e761616a1a761295 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 12:23:47 +0200 Subject: [PATCH 09/19] Fix test failures --- app/models/ci/pipeline.rb | 2 +- spec/features/projects/pipelines_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 92fae78fe4e..6820b2d41a7 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -21,7 +21,7 @@ module Ci state_machine :status, initial: :created do event :queue do - transition :created => :pending + transition created: :pending transition any - [:created, :pending] => :running end diff --git a/spec/features/projects/pipelines_spec.rb b/spec/features/projects/pipelines_spec.rb index b57652b3ea2..29d150bc597 100644 --- a/spec/features/projects/pipelines_spec.rb +++ b/spec/features/projects/pipelines_spec.rb @@ -64,7 +64,7 @@ describe "Pipelines" do before { click_link('Retry') } it { expect(page).not_to have_link('Retry') } - it { expect(page).to have_selector('.ci-pending') } + it { expect(page).to have_selector('.ci-running') } end end From ea4ac578534d3a233c3525bf8351eb2045f6e632 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 13:57:58 +0200 Subject: [PATCH 10/19] Use event `enqueue` instead of `queue` --- app/models/ci/build.rb | 4 ++-- app/models/ci/pipeline.rb | 20 +++++++----------- app/models/commit_status.rb | 2 +- app/services/ci/process_pipeline_service.rb | 2 +- spec/models/build_spec.rb | 6 ++++-- spec/models/ci/pipeline_spec.rb | 23 +++++++++++++++++---- 6 files changed, 34 insertions(+), 23 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 92dad9377c9..3d6c6ea3209 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -59,7 +59,7 @@ module Ci when: build.when, user: user, environment: build.environment, - status_event: 'queue' + status_event: 'enqueue' ) MergeRequests::AddTodoWhenBuildFailsService.new(build.project, nil).close(new_build) new_build @@ -102,7 +102,7 @@ module Ci def play(current_user = nil) # Try to queue a current build - if self.queue + if self.enqueue self.update(user: current_user) self else diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 6820b2d41a7..d00de56bf07 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -20,7 +20,7 @@ module Ci after_save :keep_around_commits state_machine :status, initial: :created do - event :queue do + event :enqueue do transition created: :pending transition any - [:created, :pending] => :running end @@ -224,18 +224,12 @@ module Ci def build_updated case latest_builds_status - when 'pending' - queue - when 'running' - run - when 'success' - succeed - when 'failed' - drop - when 'canceled' - cancel - when 'skipped' - skip + when 'pending' then enqueue + when 'running' then run + when 'success' then succeed + when 'failed' then drop + when 'canceled' then cancel + when 'skipped' then skip end end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index c21c8ce18db..703ca90edb6 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -26,7 +26,7 @@ class CommitStatus < ActiveRecord::Base scope :ignored, -> { where(allow_failure: true, status: [:failed, :canceled]) } state_machine :status do - event :queue do + event :enqueue do transition [:created, :skipped] => :pending end diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index 86c4823d18a..6f7610d42ba 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -37,7 +37,7 @@ module Ci return false unless Statuseable::COMPLETED_STATUSES.include?(current_status) if valid_statuses_for_when(build.when).include?(current_status) - build.queue + build.enqueue true else build.skip diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 60a221eba50..04fb074cfd8 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -886,8 +886,10 @@ describe Ci::Build, models: true do is_expected.to eq(build) end - context 'for success build' do - before { build.queue } + context 'for successful build' do + before do + build.success + end it 'creates a new build' do is_expected.to be_pending diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 28d07f67b26..950833cb219 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -143,7 +143,7 @@ describe Ci::Pipeline, models: true do expect(pipeline.reload.started_at).not_to be_nil end - it 'do not update on transitioning to success' do + it 'does not update on transitioning to success' do build.success expect(pipeline.reload.started_at).to be_nil @@ -157,7 +157,7 @@ describe Ci::Pipeline, models: true do expect(pipeline.reload.finished_at).not_to be_nil end - it 'do not update on transitioning to running' do + it 'does not update on transitioning to running' do build.run expect(pipeline.reload.finished_at).to be_nil @@ -257,14 +257,16 @@ describe Ci::Pipeline, models: true do subject { pipeline.reload.status } context 'on queuing' do - before { build.queue } + before do + build.enqueue + end it { is_expected.to eq('pending') } end context 'on run' do before do - build.queue + build.enqueue build.run end @@ -294,5 +296,18 @@ describe Ci::Pipeline, models: true do it { is_expected.to eq('canceled') } end + + context 'on failure and build retry' do + before do + build.drop + Ci::Build.retry(build) + end + + # We are changing a state: created > failed > running + # Instead of: created > failed > pending + # Since the pipeline already run, so it should not be pending anymore + + it { is_expected.to eq('running') } + end end end From a7f84d1a03978243c4fd5b8a878a4fea2b246f87 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 13:59:20 +0200 Subject: [PATCH 11/19] Improve transition between states for event `enqueue` --- app/models/ci/pipeline.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index d00de56bf07..8cfba92ae9b 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -22,7 +22,7 @@ module Ci state_machine :status, initial: :created do event :enqueue do transition created: :pending - transition any - [:created, :pending] => :running + transition [:success, :failed, :canceled, :skipped] => :running end event :run do From 6397ecd4e4e1d6170fd4b6fbb0390364567284d2 Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Fri, 12 Aug 2016 14:41:27 +0200 Subject: [PATCH 12/19] Update ruby 2.3.1 --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 2a63ee15af0..e8d54e768d3 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,7 +1,7 @@ -image: "ruby:2.3" +image: "ruby:2.3.1" cache: - key: "ruby-23" + key: "ruby-231" paths: - vendor/apt - vendor/ruby From 7cfc47432170be14f9449a77f893c4662634019d Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 15:09:35 +0200 Subject: [PATCH 13/19] Fix build play failure --- spec/models/build_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 04fb074cfd8..5980f6ddc32 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -888,7 +888,7 @@ describe Ci::Build, models: true do context 'for successful build' do before do - build.success + build.update(status: 'success') end it 'creates a new build' do From e0fc43ebe2d174c97062e79de36161914dafbb66 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Fri, 12 Aug 2016 16:43:10 +0200 Subject: [PATCH 14/19] Instrument Project.visible_to_user Because this method is a Rails scope we have to instrument it manually as regular the instrumentation methods only instrument methods defined directly on a Class or Module. --- CHANGELOG | 1 + config/initializers/metrics.rb | 3 +++ 2 files changed, 4 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 28834c1129a..6fe1720796d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -25,6 +25,7 @@ v 8.11.0 (unreleased) - Pre-create all builds for a Pipeline when the new Pipeline is created !5295 - Fix of 'Commits being passed to custom hooks are already reachable when using the UI' - Show member roles to all users on members page + - Project.visible_to_user is instrumented again - Fix awardable button mutuality loading spinners (ClemMakesApps) - Add support for using RequestStore within Sidekiq tasks via SIDEKIQ_REQUEST_STORE env variable - Optimize maximum user access level lookup in loading of notes diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb index cc8208db3c1..52522e099e7 100644 --- a/config/initializers/metrics.rb +++ b/config/initializers/metrics.rb @@ -148,6 +148,9 @@ if Gitlab::Metrics.enabled? config.instrument_methods(Gitlab::Highlight) config.instrument_instance_methods(Gitlab::Highlight) + + # This is a Rails scope so we have to instrument it manually. + config.instrument_method(Project, :visible_to_user) end GC::Profiler.enable From a52fe1648e8a91f6e2f4b3a5c966a06b7f9e01e3 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 17:34:43 +0200 Subject: [PATCH 15/19] Rename queue to enqueue in tests --- Gemfile | 1 + Gemfile.lock | 3 +++ bin/spring | 2 +- spec/models/ci/pipeline_spec.rb | 12 ++++++------ 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Gemfile b/Gemfile index 8b44b54e22c..9bcfc0302b6 100644 --- a/Gemfile +++ b/Gemfile @@ -296,6 +296,7 @@ group :development, :test do gem 'spring-commands-rspec', '~> 1.0.4' gem 'spring-commands-spinach', '~> 1.1.0' gem 'spring-commands-teaspoon', '~> 0.0.2' + gem "spring-commands-sidekiq" gem 'rubocop', '~> 0.41.2', require: false gem 'rubocop-rspec', '~> 1.5.0', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 3ba6048143c..4bd5b78a047 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -696,6 +696,8 @@ GEM spring (1.7.2) spring-commands-rspec (1.0.4) spring (>= 0.9.1) + spring-commands-sidekiq (1.0.0) + spring (>= 0.9.1) spring-commands-spinach (1.1.0) spring (>= 0.9.1) spring-commands-teaspoon (0.0.2) @@ -955,6 +957,7 @@ DEPENDENCIES spinach-rerun-reporter (~> 0.0.2) spring (~> 1.7.0) spring-commands-rspec (~> 1.0.4) + spring-commands-sidekiq spring-commands-spinach (~> 1.1.0) spring-commands-teaspoon (~> 0.0.2) sprockets (~> 3.6.0) diff --git a/bin/spring b/bin/spring index e0d140fe0c7..7fe232c3aae 100755 --- a/bin/spring +++ b/bin/spring @@ -3,7 +3,7 @@ # This file loads spring without using Bundler, in order to be fast. # It gets overwritten when you run the `spring binstub` command. -unless (defined?(Spring) || ENV['ENABLE_SPRING'] != '1') && File.basename($0) != 'spring' +unless defined?(Spring) require 'rubygems' require 'bundler' diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 19f1aacaabc..8137e9f8f71 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -335,8 +335,8 @@ describe Ci::Pipeline, models: true do context 'with multiple builds' do context 'when build is queued' do before do - build_a.queue - build_b.queue + build_a.enqueue + build_b.enqueue end it 'receive a pending event once' do @@ -346,9 +346,9 @@ describe Ci::Pipeline, models: true do context 'when build is run' do before do - build_a.queue + build_a.enqueue build_a.run - build_b.queue + build_b.enqueue build_b.run end @@ -382,8 +382,8 @@ describe Ci::Pipeline, models: true do let(:enabled) { false } before do - build_a.queue - build_b.queue + build_a.enqueue + build_b.enqueue end it 'did not execute pipeline_hook after touched' do From 8af6bea81ca696427faddec5c8e0d5a25c7e2d22 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 17:52:02 +0200 Subject: [PATCH 16/19] Added documentation for pipeline hooks --- doc/web_hooks/web_hooks.md | 168 +++++++++++++++++++++++++++++++++++++ 1 file changed, 168 insertions(+) diff --git a/doc/web_hooks/web_hooks.md b/doc/web_hooks/web_hooks.md index d4b28d875cd..33c1a79d59c 100644 --- a/doc/web_hooks/web_hooks.md +++ b/doc/web_hooks/web_hooks.md @@ -754,6 +754,174 @@ X-Gitlab-Event: Wiki Page Hook } ``` +## Pipeline events + +Triggered on status change of Pipeline. + +**Request Header**: + +``` +X-Gitlab-Event: Pipeline Hook +``` + +**Request Body**: + +```json +{ + "object_kind": "pipeline", + "object_attributes":{ + "id": 31, + "ref": "master", + "tag": false, + "sha": "bcbb5ec396a2c0f828686f14fac9b80b780504f2", + "before_sha": "bcbb5ec396a2c0f828686f14fac9b80b780504f2", + "status": "success", + "stages":[ + "build", + "test", + "deploy" + ], + "created_at": "2016-08-12 15:23:28 UTC", + "finished_at": "2016-08-12 15:26:29 UTC", + "duration": 63 + }, + "user":{ + "name": "Administrator", + "username": "root", + "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon" + }, + "project":{ + "name": "Gitlab Test", + "description": "Atque in sunt eos similique dolores voluptatem.", + "web_url": "http://192.168.64.1:3005/gitlab-org/gitlab-test", + "avatar_url": null, + "git_ssh_url": "git@192.168.64.1:gitlab-org/gitlab-test.git", + "git_http_url": "http://192.168.64.1:3005/gitlab-org/gitlab-test.git", + "namespace": "Gitlab Org", + "visibility_level": 20, + "path_with_namespace": "gitlab-org/gitlab-test", + "default_branch": "master" + }, + "commit":{ + "id": "bcbb5ec396a2c0f828686f14fac9b80b780504f2", + "message": "test\n", + "timestamp": "2016-08-12T17:23:21+02:00", + "url": "http://example.com/gitlab-org/gitlab-test/commit/bcbb5ec396a2c0f828686f14fac9b80b780504f2", + "author":{ + "name": "User", + "email": "user@gitlab.com" + } + }, + "builds":[ + { + "id": 380, + "stage": "deploy", + "name": "production", + "status": "skipped", + "created_at": "2016-08-12 15:23:28 UTC", + "started_at": null, + "finished_at": null, + "when": "manual", + "manual": true, + "user":{ + "name": "Administrator", + "username": "root", + "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon" + }, + "runner": null, + "artifacts_file":{ + "filename": null, + "size": null + } + }, + { + "id": 377, + "stage": "test", + "name": "test-image", + "status": "success", + "created_at": "2016-08-12 15:23:28 UTC", + "started_at": "2016-08-12 15:26:12 UTC", + "finished_at": null, + "when": "on_success", + "manual": false, + "user":{ + "name": "Administrator", + "username": "root", + "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon" + }, + "runner": null, + "artifacts_file":{ + "filename": null, + "size": null + } + }, + { + "id": 378, + "stage": "test", + "name": "test-build", + "status": "success", + "created_at": "2016-08-12 15:23:28 UTC", + "started_at": "2016-08-12 15:26:12 UTC", + "finished_at": "2016-08-12 15:26:29 UTC", + "when": "on_success", + "manual": false, + "user":{ + "name": "Administrator", + "username": "root", + "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon" + }, + "runner": null, + "artifacts_file":{ + "filename": null, + "size": null + } + }, + { + "id": 376, + "stage": "build", + "name": "build-image", + "status": "success", + "created_at": "2016-08-12 15:23:28 UTC", + "started_at": "2016-08-12 15:24:56 UTC", + "finished_at": "2016-08-12 15:25:26 UTC", + "when": "on_success", + "manual": false, + "user":{ + "name": "Administrator", + "username": "root", + "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon" + }, + "runner": null, + "artifacts_file":{ + "filename": null, + "size": null + } + }, + { + "id": 379, + "stage": "deploy", + "name": "staging", + "status": "created", + "created_at": "2016-08-12 15:23:28 UTC", + "started_at": null, + "finished_at": null, + "when": "on_success", + "manual": false, + "user":{ + "name": "Administrator", + "username": "root", + "avatar_url": "http://www.gravatar.com/avatar/e32bd13e2add097461cb96824b7a829c?s=80\u0026d=identicon" + }, + "runner": null, + "artifacts_file":{ + "filename": null, + "size": null + } + } + ] +} +``` + #### Example webhook receiver If you want to see GitLab's webhooks in action for testing purposes you can use From f7abe46109c5e90ff2ac0a8f812ea43016b5c2bc Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 21:58:28 +0200 Subject: [PATCH 17/19] Remove changes not related to this MR --- Gemfile | 1 - Gemfile.lock | 3 --- bin/spring | 2 +- 3 files changed, 1 insertion(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index 9bcfc0302b6..8b44b54e22c 100644 --- a/Gemfile +++ b/Gemfile @@ -296,7 +296,6 @@ group :development, :test do gem 'spring-commands-rspec', '~> 1.0.4' gem 'spring-commands-spinach', '~> 1.1.0' gem 'spring-commands-teaspoon', '~> 0.0.2' - gem "spring-commands-sidekiq" gem 'rubocop', '~> 0.41.2', require: false gem 'rubocop-rspec', '~> 1.5.0', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 4bd5b78a047..3ba6048143c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -696,8 +696,6 @@ GEM spring (1.7.2) spring-commands-rspec (1.0.4) spring (>= 0.9.1) - spring-commands-sidekiq (1.0.0) - spring (>= 0.9.1) spring-commands-spinach (1.1.0) spring (>= 0.9.1) spring-commands-teaspoon (0.0.2) @@ -957,7 +955,6 @@ DEPENDENCIES spinach-rerun-reporter (~> 0.0.2) spring (~> 1.7.0) spring-commands-rspec (~> 1.0.4) - spring-commands-sidekiq spring-commands-spinach (~> 1.1.0) spring-commands-teaspoon (~> 0.0.2) sprockets (~> 3.6.0) diff --git a/bin/spring b/bin/spring index 7fe232c3aae..e0d140fe0c7 100755 --- a/bin/spring +++ b/bin/spring @@ -3,7 +3,7 @@ # This file loads spring without using Bundler, in order to be fast. # It gets overwritten when you run the `spring binstub` command. -unless defined?(Spring) +unless (defined?(Spring) || ENV['ENABLE_SPRING'] != '1') && File.basename($0) != 'spring' require 'rubygems' require 'bundler' From 5822a333d4c1bb43304c781f3a8b8d3eb99861a8 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 22:09:26 +0200 Subject: [PATCH 18/19] Capitalise URL on web_hooks/form --- app/views/shared/web_hooks/_form.html.haml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml index a672c28de39..d2ec6c3ddef 100644 --- a/app/views/shared/web_hooks/_form.html.haml +++ b/app/views/shared/web_hooks/_form.html.haml @@ -29,56 +29,56 @@ = f.label :push_events, class: 'list-label' do %strong Push events %p.light - This url will be triggered by a push to the repository + This URL will be triggered by a push to the repository %li = f.check_box :tag_push_events, class: 'pull-left' .prepend-left-20 = f.label :tag_push_events, class: 'list-label' do %strong Tag push events %p.light - This url will be triggered when a new tag is pushed to the repository + This URL will be triggered when a new tag is pushed to the repository %li = f.check_box :note_events, class: 'pull-left' .prepend-left-20 = f.label :note_events, class: 'list-label' do %strong Comments %p.light - This url will be triggered when someone adds a comment + This URL will be triggered when someone adds a comment %li = f.check_box :issues_events, class: 'pull-left' .prepend-left-20 = f.label :issues_events, class: 'list-label' do %strong Issues events %p.light - This url will be triggered when an issue is created/updated/merged + This URL will be triggered when an 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 %strong Merge Request events %p.light - This url will be triggered when a merge request is created/updated/merged + This URL will be triggered when a merge request is created/updated/merged %li = f.check_box :build_events, class: 'pull-left' .prepend-left-20 = f.label :build_events, class: 'list-label' do %strong Build events %p.light - This url will be triggered when the build status changes + This URL will be triggered when the build status changes %li = f.check_box :pipeline_events, class: 'pull-left' .prepend-left-20 = f.label :pipeline_events, class: 'list-label' do %strong Pipeline events %p.light - This url will be triggered when the pipeline status changes + This URL will be triggered when the pipeline status changes %li = f.check_box :wiki_page_events, class: 'pull-left' .prepend-left-20 = f.label :wiki_page_events, class: 'list-label' do %strong Wiki Page events %p.light - This url will be triggered when a wiki page is created/updated + This URL will be triggered when a wiki page is created/updated .form-group = f.label :enable_ssl_verification, "SSL verification", class: 'label-light checkbox' .checkbox From cd4d8b91eb77b12a168bbca3a7ee9ec8bf1fba5e Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 12 Aug 2016 22:09:58 +0200 Subject: [PATCH 19/19] Make explicit call for all event types for ProjectHook factory --- spec/factories/project_hooks.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb index c709432c865..4fd51a23490 100644 --- a/spec/factories/project_hooks.rb +++ b/spec/factories/project_hooks.rb @@ -7,15 +7,13 @@ FactoryGirl.define do end trait :all_events_enabled do - %w[push_events - merge_requests_events - tag_push_events - issues_events - note_events - build_events - pipeline_events].each do |event| - send(event, true) - end + push_events true + merge_requests_events true + tag_push_events true + issues_events true + note_events true + build_events true + pipeline_events true end end end