Merge branch 'master' of https://gitlab.com/gitlab-org/gitlab-ce into 2451-fix-mentions-in-issue-updates

38 files changed, 451 insertions, 163 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
diff --git a/CHANGELOG b/CHANGELOG
index 28834c1129a..96965a20f69 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
@@ -92,6 +93,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/assets/stylesheets/pages/environments.scss b/app/assets/stylesheets/pages/environments.scss
index e160d676e35..55f9d4a0011 100644
--- a/app/assets/stylesheets/pages/environments.scss
+++ b/app/assets/stylesheets/pages/environments.scss
@@ -1,5 +1,35 @@
 .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;
+    }
+  }
+
+  .branch-name {
+    color: $gl-dark-link-color;
+  }
+}
+
+.table.builds.environments {
+  min-width: 500px;
+
+  .icon-container {
+    width: 20px;
+    text-align: center;
+  }
 }
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/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb
index 2160cf7a690..aa8acbe7567 100644
--- a/app/helpers/avatars_helper.rb
+++ b/app/helpers/avatars_helper.rb
@@ -7,8 +7,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/models/ci/build.rb b/app/models/ci/build.rb
index 88a340379b8..3d6c6ea3209 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -42,24 +42,25 @@ module Ci
       end
 
       def retry(build, user = nil)
-        new_build = Ci::Build.new(status: 'pending')
-        new_build.ref = build.ref
-        new_build.tag = build.tag
-        new_build.options = build.options
-        new_build.commands = build.commands
-        new_build.tag_list = build.tag_list
-        new_build.project = build.project
-        new_build.pipeline = build.pipeline
-        new_build.name = build.name
-        new_build.allow_failure = build.allow_failure
-        new_build.stage = build.stage
-        new_build.stage_idx = build.stage_idx
-        new_build.trigger_request = build.trigger_request
-        new_build.yaml_variables = build.yaml_variables
-        new_build.when = build.when
-        new_build.user = user
-        new_build.environment = build.environment
-        new_build.save
+        new_build = Ci::Build.create(
+          ref: build.ref,
+          tag: build.tag,
+          options: build.options,
+          commands: build.commands,
+          tag_list: build.tag_list,
+          project: build.project,
+          pipeline: build.pipeline,
+          name: build.name,
+          allow_failure: build.allow_failure,
+          stage: build.stage,
+          stage_idx: build.stage_idx,
+          trigger_request: build.trigger_request,
+          yaml_variables: build.yaml_variables,
+          when: build.when,
+          user: user,
+          environment: build.environment,
+          status_event: 'enqueue'
+        )
         MergeRequests::AddTodoWhenBuildFailsService.new(build.project, nil).close(new_build)
         new_build
       end
@@ -101,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 718fe3290c1..8cfba92ae9b 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -19,6 +19,45 @@ module Ci
 
     after_save :keep_around_commits
 
+    state_machine :status, initial: :created do
+      event :enqueue do
+        transition created: :pending
+        transition [:success, :failed, :canceled, :skipped] => :running
+      end
+
+      event :run do
+        transition any => :running
+      end
+
+      event :skip do
+        transition any => :skipped
+      end
+
+      event :drop do
+        transition any => :failed
+      end
+
+      event :succeed do
+        transition any => :success
+      end
+
+      event :cancel do
+        transition any => :canceled
+      end
+
+      before_transition [:created, :pending] => :running do |pipeline|
+        pipeline.started_at = Time.now
+      end
+
+      before_transition any => [:success, :failed, :canceled] do |pipeline|
+        pipeline.finished_at = Time.now
+      end
+
+      before_transition do |pipeline|
+        pipeline.update_duration
+      end
+    end
+
     # ref can't be HEAD or SHA, can only be branch/tag name
     scope :latest_successful_for, ->(ref = default_branch) do
       where(ref: ref).success.order(id: :desc).limit(1)
@@ -89,16 +128,12 @@ module Ci
 
     def cancel_running
       builds.running_or_pending.each(&:cancel)
-
-      reload_status!
     end
 
     def retry_failed(user)
       builds.latest.failed.select(&:retryable?).each do |build|
         Ci::Build.retry(build, user)
       end
-
-      reload_status!
     end
 
     def latest?
@@ -185,7 +220,17 @@ module Ci
 
     def process!
       Ci::ProcessPipelineService.new(project, user).execute(self)
-      reload_status!
+    end
+
+    def build_updated
+      case latest_builds_status
+      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
 
     def predefined_variables
@@ -194,22 +239,18 @@ module Ci
       ]
     end
 
-    def reload_status!
-      statuses.reload
-      self.status =
-        if yaml_errors.blank?
-          statuses.latest.status || 'skipped'
-        else
-          'failed'
-        end
-      self.started_at = statuses.started_at
-      self.finished_at = statuses.finished_at
+    def update_duration
       self.duration = statuses.latest.duration
-      save
     end
 
     private
 
+    def latest_builds_status
+      return 'failed' unless yaml_errors.blank?
+
+      statuses.latest.status || 'skipped'
+    end
+
     def keep_around_commits
       return unless project
 
diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 20713314a25..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
 
@@ -62,6 +62,17 @@ class CommitStatus < ActiveRecord::Base
       commit_status.update_attributes finished_at: Time.now
     end
 
+    # We use around_transition to process pipeline on next stages as soon as possible, before the `after_*` is executed
+    around_transition any => [:success, :failed, :canceled] do |commit_status, block|
+      block.call
+
+      commit_status.pipeline.try(:process!)
+    end
+
+    after_transition do |commit_status, transition|
+      commit_status.pipeline.try(:build_updated) unless transition.loopback?
+    end
+
     after_transition [:created, :pending, :running] => :success do |commit_status|
       MergeRequests::MergeWhenBuildSucceedsService.new(commit_status.pipeline.project, nil).trigger(commit_status)
     end
@@ -69,13 +80,6 @@ class CommitStatus < ActiveRecord::Base
     after_transition any => :failed do |commit_status|
       MergeRequests::AddTodoWhenBuildFailsService.new(commit_status.pipeline.project, nil).execute(commit_status)
     end
-
-    # We use around_transition to process pipeline on next stages as soon as possible, before the `after_*` is executed
-    around_transition any => [:success, :failed, :canceled] do |commit_status, block|
-      block.call
-
-      commit_status.pipeline.process! if commit_status.pipeline
-    end
   end
 
   delegate :sha, :short_sha, to: :pipeline
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/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index 7398fd8e10a..cde856b0186 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -37,7 +37,8 @@ module Ci
       end
 
       if !ignore_skip_ci && skip_ci?
-        return error('Creation of pipeline is skipped', save: save_on_errors)
+        pipeline.skip if save_on_errors
+        return pipeline
       end
 
       unless pipeline.config_builds_attributes.present?
@@ -93,7 +94,7 @@ module Ci
 
     def error(message, save: false)
       pipeline.errors.add(:base, message)
-      pipeline.reload_status! if save
+      pipeline.drop if save
       pipeline
     end
   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/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/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
diff --git a/app/views/projects/deployments/_commit.html.haml b/app/views/projects/deployments/_commit.html.haml
index 0f9d9512d88..28813babd7b 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"
-    &middot;
+    .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
+        = 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/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 e2453395602..36a6162a5a8 100644
--- a/app/views/projects/environments/_environment.html.haml
+++ b/app/views/projects/environments/_environment.html.haml
@@ -2,8 +2,12 @@
 
 %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
+      = user_avatar(user: last_deployment.user, size: 20)
+      %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 a6dd34653ab..b3eb5b0011a 100644
--- a/app/views/projects/environments/index.html.haml
+++ b/app/views/projects/environments/index.html.haml
@@ -23,10 +23,11 @@
           New environment
   - else
     .table-holder
-      %table.table.environments
+      %table.table.builds.environments
         %tbody
           %th Environment
-          %th Last deployment
-          %th Date
+          %th Last Deployment
+          %th Commit
+          %th
           %th
         = render @environments
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
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/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
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/features/steps/shared/builds.rb b/features/steps/shared/builds.rb
index c7f61da05fa..70e6d4836b2 100644
--- a/features/steps/shared/builds.rb
+++ b/features/steps/shared/builds.rb
@@ -12,7 +12,6 @@ module SharedBuilds
   step 'project has a recent build' do
     @pipeline = create(:ci_empty_pipeline, project: @project, sha: @project.commit.sha, ref: 'master')
     @build = create(:ci_build_with_coverage, pipeline: @pipeline)
-    @pipeline.reload_status!
   end
 
   step 'recent build is successful' do
@@ -24,8 +23,7 @@ module SharedBuilds
   end
 
   step 'project has another build that is running' do
-    create(:ci_build, pipeline: @pipeline, name: 'second build', status: 'running')
-    @pipeline.reload_status!
+    create(:ci_build, pipeline: @pipeline, name: 'second build', status_event: 'run')
   end
 
   step 'I visit recent build details page' do
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/features/projects/pipelines_spec.rb b/spec/features/projects/pipelines_spec.rb
index c39bb7ffa37..29d150bc597 100644
--- a/spec/features/projects/pipelines_spec.rb
+++ b/spec/features/projects/pipelines_spec.rb
@@ -12,7 +12,7 @@ describe "Pipelines" do
   end
 
   describe 'GET /:project/pipelines' do
-    let!(:pipeline) { create(:ci_pipeline, project: project, ref: 'master', status: 'running') }
+    let!(:pipeline) { create(:ci_empty_pipeline, project: project, ref: 'master', status: 'running') }
 
     [:all, :running, :branches].each do |scope|
       context "displaying #{scope}" do
@@ -31,10 +31,10 @@ describe "Pipelines" do
     end
 
     context 'cancelable pipeline' do
-      let!(:running) { create(:ci_build, :running, pipeline: pipeline, stage: 'test', commands: 'test') }
+      let!(:build) { create(:ci_build, pipeline: pipeline, stage: 'test', commands: 'test') }
 
       before do
-        pipeline.reload_status!
+        build.run
         visit namespace_project_pipelines_path(project.namespace, project)
       end
 
@@ -50,10 +50,10 @@ describe "Pipelines" do
     end
 
     context 'retryable pipelines' do
-      let!(:failed) { create(:ci_build, :failed, pipeline: pipeline, stage: 'test', commands: 'test') }
+      let!(:build) { create(:ci_build, pipeline: pipeline, stage: 'test', commands: 'test') }
 
       before do
-        pipeline.reload_status!
+        build.drop
         visit namespace_project_pipelines_path(project.namespace, project)
       end
 
@@ -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
 
@@ -87,7 +87,6 @@ describe "Pipelines" do
         let!(:running) { create(:generic_commit_status, status: 'running', pipeline: pipeline, stage: 'test') }
 
         before do
-          pipeline.reload_status!
           visit namespace_project_pipelines_path(project.namespace, project)
         end
 
@@ -101,10 +100,10 @@ describe "Pipelines" do
       end
 
       context 'when failed' do
-        let!(:failed) { create(:generic_commit_status, status: 'failed', pipeline: pipeline, stage: 'test') }
+        let!(:status) { create(:generic_commit_status, :pending, pipeline: pipeline, stage: 'test') }
 
         before do
-          pipeline.reload_status!
+          status.drop
           visit namespace_project_pipelines_path(project.namespace, project)
         end
 
diff --git a/spec/lib/ci/charts_spec.rb b/spec/lib/ci/charts_spec.rb
index 2cd6b00dad6..fb6cc398307 100644
--- a/spec/lib/ci/charts_spec.rb
+++ b/spec/lib/ci/charts_spec.rb
@@ -2,22 +2,23 @@ require 'spec_helper'
 
 describe Ci::Charts, lib: true do
   context "build_times" do
+    let(:project) { create(:empty_project) }
+    let(:chart) { Ci::Charts::BuildTime.new(project) }
+
+    subject { chart.build_times }
+
     before do
-      @pipeline = FactoryGirl.create(:ci_pipeline)
-      FactoryGirl.create(:ci_build, pipeline: @pipeline)
-      @pipeline.reload_status!
+      create(:ci_empty_pipeline, project: project, duration: 120)
     end
 
     it 'returns build times in minutes' do
-      chart = Ci::Charts::BuildTime.new(@pipeline.project)
-      expect(chart.build_times).to eq([2])
+      is_expected.to contain_exactly(2)
     end
 
     it 'handles nil build times' do
-      create(:ci_pipeline, duration: nil, project: @pipeline.project)
+      create(:ci_empty_pipeline, project: project, duration: nil)
 
-      chart = Ci::Charts::BuildTime.new(@pipeline.project)
-      expect(chart.build_times).to eq([2, 0])
+      is_expected.to contain_exactly(2, 0)
     end
   end
 end
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 60a221eba50..5980f6ddc32 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.update(status: '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 fdb579ab45c..950833cb219 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Ci::Pipeline, models: true do
   let(:project) { FactoryGirl.create :empty_project }
-  let(:pipeline) { FactoryGirl.create :ci_pipeline, project: project }
+  let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project }
 
   it { is_expected.to belong_to(:project) }
   it { is_expected.to belong_to(:user) }
@@ -51,25 +51,6 @@ describe Ci::Pipeline, models: true do
     end
   end
 
-  describe "#finished_at" do
-    let(:pipeline) { FactoryGirl.create :ci_pipeline }
-
-    it "returns finished_at of latest build" do
-      build = FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 60
-      FactoryGirl.create :ci_build, pipeline: pipeline, finished_at: Time.now - 120
-      pipeline.reload_status!
-
-      expect(pipeline.finished_at.to_i).to eq(build.finished_at.to_i)
-    end
-
-    it "returns nil if there is no finished build" do
-      FactoryGirl.create :ci_not_started_build, pipeline: pipeline
-      pipeline.reload_status!
-
-      expect(pipeline.finished_at).to be_nil
-    end
-  end
-
   describe "coverage" do
     let(:project) { FactoryGirl.create :empty_project, build_coverage_regex: "/.*/" }
     let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, project: project }
@@ -139,32 +120,47 @@ describe Ci::Pipeline, models: true do
     end
   end
 
-  describe '#reload_status!' do
-    let(:pipeline) { create :ci_empty_pipeline, project: project }
+  describe 'state machine' do
+    let(:current) { Time.now.change(usec: 0) }
+    let(:build) { create :ci_build, name: 'build1', pipeline: pipeline, started_at: current - 60, finished_at: current }
+    let(:build2) { create :ci_build, name: 'build2', pipeline: pipeline, started_at: current - 60, finished_at: current }
 
-    context 'dependent objects' do
-      let(:commit_status) { create :commit_status, :pending, pipeline: pipeline }
+    describe '#duration' do
+      before do
+        build.skip
+        build2.skip
+      end
 
-      it 'executes reload_status! after succeeding dependent object' do
-        expect(pipeline).to receive(:reload_status!).and_return(true)
+      it 'matches sum of builds duration' do
+        expect(pipeline.reload.duration).to eq(build.duration + build2.duration)
+      end
+    end
+
+    describe '#started_at' do
+      it 'updates on transitioning to running' do
+        build.run
 
-        commit_status.success
+        expect(pipeline.reload.started_at).not_to be_nil
+      end
+
+      it 'does not update on transitioning to success' do
+        build.success
+
+        expect(pipeline.reload.started_at).to be_nil
       end
     end
 
-    context 'updates' do
-      let(:current) { Time.now.change(usec: 0) }
-      let(:build) { FactoryGirl.create :ci_build, pipeline: pipeline, started_at: current - 120, finished_at: current - 60 }
+    describe '#finished_at' do
+      it 'updates on transitioning to success' do
+        build.success
 
-      before do
-        build
-        pipeline.reload_status!
+        expect(pipeline.reload.finished_at).not_to be_nil
       end
 
-      [:status, :started_at, :finished_at, :duration].each do |param|
-        it "#{param}" do
-          expect(pipeline.send(param)).to eq(build.send(param))
-        end
+      it 'does not update on transitioning to running' do
+        build.run
+
+        expect(pipeline.reload.finished_at).to be_nil
       end
     end
   end
@@ -254,4 +250,64 @@ describe Ci::Pipeline, models: true do
       end
     end
   end
+
+  describe '#status' do
+    let!(:build) { create(:ci_build, :created, pipeline: pipeline, name: 'test') }
+
+    subject { pipeline.reload.status }
+
+    context 'on queuing' do
+      before do
+        build.enqueue
+      end
+
+      it { is_expected.to eq('pending') }
+    end
+
+    context 'on run' do
+      before do
+        build.enqueue
+        build.run
+      end
+
+      it { is_expected.to eq('running') }
+    end
+
+    context 'on drop' do
+      before do
+        build.drop
+      end
+
+      it { is_expected.to eq('failed') }
+    end
+
+    context 'on success' do
+      before do
+        build.success
+      end
+
+      it { is_expected.to eq('success') }
+    end
+
+    context 'on cancel' do
+      before do
+        build.cancel
+      end
+
+      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
diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb
index a4cdd8f3140..41503885dd9 100644
--- a/spec/requests/api/builds_spec.rb
+++ b/spec/requests/api/builds_spec.rb
@@ -9,7 +9,7 @@ describe API::API, api: true do
   let!(:developer) { create(:project_member, :developer, user: user, project: project) }
   let(:reporter) { create(:project_member, :reporter, project: project) }
   let(:guest) { create(:project_member, :guest, project: project) }
-  let!(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) }
+  let!(:pipeline) { create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) }
   let!(:build) { create(:ci_build, pipeline: pipeline) }
 
   describe 'GET /projects/:id/builds ' do
@@ -174,7 +174,11 @@ describe API::API, api: true do
 
   describe 'GET /projects/:id/artifacts/:ref_name/download?job=name' do
     let(:api_user) { reporter.user }
-    let(:build) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
+    let(:build) { create(:ci_build, :artifacts, pipeline: pipeline) }
+
+    before do
+      build.success
+    end
 
     def path_for_ref(ref = pipeline.ref, job = build.name)
       api("/projects/#{project.id}/builds/artifacts/#{ref}/download?job=#{job}", api_user)
@@ -238,10 +242,6 @@ describe API::API, api: true do
         it { expect(response.headers).to include(download_headers) }
       end
 
-      before do
-        pipeline.reload_status!
-      end
-
       context 'with regular branch' do
         before do
           pipeline.update(ref: 'master',
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/ci/image_for_build_service_spec.rb b/spec/services/ci/image_for_build_service_spec.rb
index 259062406c7..c931c3e4829 100644
--- a/spec/services/ci/image_for_build_service_spec.rb
+++ b/spec/services/ci/image_for_build_service_spec.rb
@@ -14,7 +14,6 @@ module Ci
       context 'branch name' do
         before { allow(project).to receive(:commit).and_return(OpenStruct.new(sha: commit_sha)) }
         before { build.run! }
-        before { pipeline.reload_status! }
         let(:image) { service.execute(project, ref: 'master') }
 
         it { expect(image).to be_kind_of(OpenStruct) }
@@ -32,7 +31,6 @@ module Ci
 
       context 'commit sha' do
         before { build.run! }
-        before { pipeline.reload_status! }
         let(:image) { service.execute(project, sha: build.sha) }
 
         it { expect(image).to be_kind_of(OpenStruct) }
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)
+  shared_examples 'group destruction' do |async|
+    context 'database records' do
+      before do
+        destroy_group(group, user, async)
+      end
+
+      it { expect(Group.all).not_to include(group) }
+      it { expect(Project.all).not_to include(project) }
     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
+        before do
+          # Run sidekiq immediatly to check that renamed dir will be removed
+          Sidekiq::Testing.inline! { destroy_group(group, user, async) }
+        end
 
-  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) }
+        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_falsey }
-    end
+      context 'Sidekiq fake' do
+        before do
+          # Dont run sidekiq to check if renamed repository exists
+          Sidekiq::Testing.fake! { destroy_group(group, user, async) }
+        end
 
-    context 'Sidekiq fake' do
-      before do
-        # Dont run sidekiq to check if renamed repository exists
-        Sidekiq::Testing.fake! { destroy_group(group, user) }
+        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
 
-      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 }
+    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