Refactor code to use available and stopped statuses and refactor views to use separate renders

19 files changed, 111 insertions, 94 deletions

diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb
index 2ec316a1ebd..40da5be2e49 100644
--- a/app/controllers/projects/environments_controller.rb
+++ b/app/controllers/projects/environments_controller.rb
@@ -10,8 +10,8 @@ class Projects::EnvironmentsController < Projects::ApplicationController
     @all_environments = project.environments
     @environments =
       case @scope
-        when 'closed' then @all_environments.closed
-        else @all_environments.opened
+        when 'stopped' then @all_environments.stopped
+        else @all_environments.available
       end
   end
 
@@ -45,7 +45,9 @@ class Projects::EnvironmentsController < Projects::ApplicationController
   end
 
   def stop
-
+    action = @environment.stop_action
+    new_action = action.active? ? action : action.play(current_user)
+    redirect_to [project.namespace.become(Namespace), project, new_action]
   end
 
   def destroy
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 1c1938f957b..86e12660c23 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -416,7 +416,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
             id: environment.id,
             name: environment.name,
             url: namespace_project_environment_path(project.namespace, project, environment),
-            stop_url: (stop_namespace_project_environment_path(project.namespace, project, environment) if environment.closeable?),
+            stop_url: (stop_namespace_project_environment_path(project.namespace, project, environment) if environment.stoppable?),
             external_url: environment.external_url,
             external_url_formatted: environment.formatted_external_url,
             deployed_at: deployment.try(:created_at),
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 6f3e83976e7..87475119b23 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -106,14 +106,6 @@ module Ci
       project.builds_enabled? && commands.present? && manual? && skipped?
     end
 
-    def close_environment?
-      options.fetch(:environment, {}).fetch(:close, false)
-    end
-
-    def closes_environment?(name)
-      environment == name && close_environment?
-    end
-
     def play(current_user = nil)
       # Try to queue a current build
       if self.enqueue
diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index 054d54f124e..18d9e96301c 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -11,6 +11,8 @@ class Deployment < ActiveRecord::Base
 
   delegate :name, to: :environment, prefix: true
 
+  store :properties, accessors: [:on_stop]
+
   after_save :create_ref
 
   def commit
@@ -34,7 +36,7 @@ class Deployment < ActiveRecord::Base
   end
 
   def manual_actions
-    deployable.try(:other_actions)
+    @manual_actions ||= deployable.try(:other_actions)
   end
 
   def includes_commit?(commit)
@@ -84,17 +86,14 @@ class Deployment < ActiveRecord::Base
       take
   end
 
-  def close_action
+  def stop_action
     return nil unless manual_actions
 
-    @close_action ||=
-      manual_actions.find do |manual_action|
-        manual_action.try(:closes_environment?, deployable.environment)
-      end
+    @stop_action ||= manual_actions.find_by(name: on_stop)
   end
 
-  def closeable?
-    close_action.present?
+  def stoppable?
+    on_stop.present? && stop_action.present?
   end
 
   def formatted_deployment_time
diff --git a/app/models/environment.rb b/app/models/environment.rb
index 07f14a7ad8d..93e7dedd6f8 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -19,22 +19,22 @@ class Environment < ActiveRecord::Base
             allow_nil: true,
             addressable_url: true
 
-  delegate :closeable?, :close_action, to: :last_deployment, allow_nil: true
+  delegate :stoppable?, :stop_action, to: :last_deployment, allow_nil: true
 
-  scope :opened, -> { where(state: [:opened]) }
-  scope :closed, -> { where(state: [:closed]) }
+  scope :available, -> { where(state: [:available]) }
+  scope :stopped, -> { where(state: [:stopped]) }
 
-  state_machine :state, initial: :opened do
-    event :close do
-      transition opened: :closed
+  state_machine :state, initial: :available do
+    event :start do
+      transition stopped: :available
     end
 
-    event :reopen do
-      transition closed: :opened
+    event :stop do
+      transition available: :stopped
     end
 
-    state :opened
-    state :closed
+    state :available
+    state :stopped
   end
 
   def last_deployment
diff --git a/app/models/project.rb b/app/models/project.rb
index 2e8073733d4..197edabe821 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1292,7 +1292,7 @@ class Project < ActiveRecord::Base
         environment_ids.where(ref: ref)
       end
 
-    environments.opened.where(id: environment_ids).select do |environment|
+    environments.available.where(id: environment_ids).select do |environment|
       environment.includes_commit?(commit)
     end
   end
diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb
index 923de58b244..47c740addb0 100644
--- a/app/services/create_deployment_service.rb
+++ b/app/services/create_deployment_service.rb
@@ -8,12 +8,12 @@ class CreateDeploymentService < BaseService
       @deployable = deployable
       @environment = prepare_environment
 
-      if close?
-        @environment.close
+      if stop?
+        @environment.stop
         return
       end
 
-      @environment.reopen
+      @environment.start
 
       deploy.tap do |deployment|
         deployment.update_merge_request_metrics!
@@ -61,10 +61,6 @@ class CreateDeploymentService < BaseService
     options[:url]
   end
 
-  def close?
-    options[:close]
-  end
-
   def options
     params[:options] || {}
   end
@@ -72,4 +68,8 @@ class CreateDeploymentService < BaseService
   def variables
     params[:variables] || []
   end
+
+  def stop?
+    params[:options].fetch(:stop, false)
+  end
 end
diff --git a/app/views/projects/deployments/_actions.haml b/app/views/projects/deployments/_actions.haml
index 387ff018073..58a214bdbd1 100644
--- a/app/views/projects/deployments/_actions.haml
+++ b/app/views/projects/deployments/_actions.haml
@@ -1,33 +1,15 @@
-- if can?(current_user, :create_deployment, deployment) && deployment.deployable
-  .pull-right
-  
-    - external_url = deployment.environment.external_url
-    - if external_url
-      = link_to external_url, target: '_blank', class: 'btn external-url' do
-        = icon('external-link')
-    
-    - actions = deployment.manual_actions
-    - if actions.present?
-      .inline
-        .dropdown
-          %a.dropdown-new.btn.btn-default{type: 'button', 'data-toggle' => 'dropdown'}
-            = custom_icon('icon_play')
-            = icon('caret-down')
-          %ul.dropdown-menu.dropdown-menu-align-right
-            - actions.each do |action|
-              %li
-                = link_to [:play, @project.namespace.becomes(Namespace), @project, action], method: :post, rel: 'nofollow' do
-                  = custom_icon('icon_play')
-                  %span= action.name.humanize
+- if can?(current_user, :create_deployment, deployment)
+  - actions = deployment.manual_actions
+  - if actions.present?
+    .inline
+      .dropdown
+        %a.dropdown-new.btn.btn-default{type: 'button', 'data-toggle' => 'dropdown'}
+          = custom_icon('icon_play')
+          = icon('caret-down')
+        %ul.dropdown-menu.dropdown-menu-align-right
+          - actions.each do |action|
+            %li
+              = link_to [:play, @project.namespace.becomes(Namespace), @project, action], method: :post, rel: 'nofollow' do
+                = custom_icon('icon_play')
+                %span= action.name.humanize
 
-      - if local_assigns.fetch(:allow_close, false) && deployment.closeable?
-        .inline
-          = link_to [:play, @project.namespace.becomes(Namespace), @project, deployment.close_action], method: :post, class: 'btn close-env-link', rel: 'nofollow', data: { confirm: 'Are you sure you want to close this environment?' } do
-            = icon('stop', class: 'close-env-icon')
-
-    - if local_assigns.fetch(:allow_rollback, false)
-      = link_to [:retry, @project.namespace.becomes(Namespace), @project, deployment.deployable], method: :post, class: 'btn' do
-        - if deployment.last?
-          Re-deploy
-        - else
-          Rollback
diff --git a/app/views/projects/deployments/_deployment.html.haml b/app/views/projects/deployments/_deployment.html.haml
index ca0005abd0c..9238f232c7e 100644
--- a/app/views/projects/deployments/_deployment.html.haml
+++ b/app/views/projects/deployments/_deployment.html.haml
@@ -17,4 +17,6 @@
     #{time_ago_with_tooltip(deployment.created_at)}
 
   %td.hidden-xs
-    = render 'projects/deployments/actions', deployment: deployment, allow_rollback: true
+    .pull-right
+      = render 'projects/deployments/actions', deployment: deployment
+      = render 'projects/deployments/rollback', deployment: deployment
diff --git a/app/views/projects/deployments/_rollback.haml b/app/views/projects/deployments/_rollback.haml
new file mode 100644
index 00000000000..5941e01c6f1
--- /dev/null
+++ b/app/views/projects/deployments/_rollback.haml
@@ -0,0 +1,6 @@
+- if can?(current_user, :create_deployment, deployment) && deployment.deployable
+  = link_to [:retry, @project.namespace.becomes(Namespace), @project, deployment.deployable], method: :post, class: 'btn btn-build' do
+    - if deployment.last?
+      Re-deploy
+    - else
+      Rollback
diff --git a/app/views/projects/environments/_environment.html.haml b/app/views/projects/environments/_environment.html.haml
index 19c536897bd..b75d5df4150 100644
--- a/app/views/projects/environments/_environment.html.haml
+++ b/app/views/projects/environments/_environment.html.haml
@@ -28,4 +28,8 @@
       #{time_ago_with_tooltip(last_deployment.created_at)}
 
   %td.hidden-xs
-    = render 'projects/deployments/actions', deployment: last_deployment, allow_close: environment.opened?
+    .pull-right
+      = render 'projects/environments/external_url', environment: environment
+      = render 'projects/deployments/actions', deployment: last_deployment
+      = render 'projects/environments/stop', environment: environment
+      = render 'projects/deployments/rollback', deployment: last_deployment
diff --git a/app/views/projects/environments/_external_url.html.haml b/app/views/projects/environments/_external_url.html.haml
new file mode 100644
index 00000000000..6255e4baea0
--- /dev/null
+++ b/app/views/projects/environments/_external_url.html.haml
@@ -0,0 +1,3 @@
+- if environment.external_url
+  = link_to environment.external_url, target: '_blank', class: 'btn external-url' do
+    = icon('external-link')
diff --git a/app/views/projects/environments/_stop.html.haml b/app/views/projects/environments/_stop.html.haml
new file mode 100644
index 00000000000..6ed6aee141b
--- /dev/null
+++ b/app/views/projects/environments/_stop.html.haml
@@ -0,0 +1,5 @@
+- if environment.available? && environment.stoppable?
+  .inline
+    = link_to stop_namespace_project_environment_path(@project.namespace, @project, environment), method: :post,
+      class: 'btn close-env-link', rel: 'nofollow', data: { confirm: 'Are you sure you want to close this environment?' } do
+      = icon('stop', class: 'close-env-icon')
diff --git a/app/views/projects/environments/index.html.haml b/app/views/projects/environments/index.html.haml
index f0b64a8775b..70185176222 100644
--- a/app/views/projects/environments/index.html.haml
+++ b/app/views/projects/environments/index.html.haml
@@ -8,14 +8,14 @@
       %li{class: ('active' if @scope.nil?)}
         = link_to project_environments_path(@project) do
           Available
-          %span.badge.js-avaibale-environments-count
-            = number_with_delimiter(@all_environments.opened.count)
+          %span.badge.js-available-environments-count
+            = number_with_delimiter(@all_environments.available.count)
 
-      %li{class: ('active' if @scope == 'closed')}
-        = link_to project_environments_path(@project, scope: :closed) do
+      %li{class: ('active' if @scope == 'stopped')}
+        = link_to project_environments_path(@project, scope: :stopped) do
           Stopped
           %span.badge.js-stopped-environments-count
-            = number_with_delimiter(@all_environments.closed.count)
+            = number_with_delimiter(@all_environments.stopped.count)
 
     .nav-controls
       - if can?(current_user, :create_environment, @project) && !@all_environments.blank?
diff --git a/app/views/projects/environments/show.html.haml b/app/views/projects/environments/show.html.haml
index c4209d499ad..3b4d0395db0 100644
--- a/app/views/projects/environments/show.html.haml
+++ b/app/views/projects/environments/show.html.haml
@@ -2,18 +2,20 @@
 - page_title "Environments"
 = render "projects/pipelines/head"
 
-- last_deployment = @environment.last_deployment
-
 %div{ class: container_class }
   .top-area
     .col-md-9
       %h3.page-title= @environment.name.capitalize
     .col-md-3
       .nav-controls
+        - if can?(current_user, :read_environmnet, @environment)
+          = render 'projects/environments/external_url', environment: @environment
+
         - if can?(current_user, :update_environment, @environment)
           = link_to 'Edit', edit_namespace_project_environment_path(@project.namespace, @project, @environment), class: 'btn'
-          - if @environment.opened? && last_deployment.try(:close_action)
-            = link_to 'Close', [:play, @project.namespace.becomes(Namespace), @project, last_deployment.close_action], data: { confirm: 'Are you sure you want to close this environment?' }, class: 'btn btn-danger', method: :post
+          - if @environment.available? && @environment.stoppable?
+            = link_to 'Stop', stop_namespace_project_environment_path(@project.namespace, @project, @environment), data: { confirm: 'Are you sure you want to close this environment?' }, class: 'btn btn-danger', method: :post
+          = link_to 'Destroy', namespace_project_environment_path(@project.namespace, @project, @environment), data: { confirm: 'Are you sure you want to delete this environment?' }, class: 'btn btn-danger', method: :delete
 
   .deployments-container
     - if @deployments.blank?
diff --git a/db/fixtures/development/14_pipelines.rb b/db/fixtures/development/14_pipelines.rb
index d3fabe111a1..08ad3097d34 100644
--- a/db/fixtures/development/14_pipelines.rb
+++ b/db/fixtures/development/14_pipelines.rb
@@ -16,8 +16,8 @@ class Gitlab::Seeder::Pipelines
     { name: 'env:alpha', stage: 'deploy', environment: 'alpha', status: :pending },
     { name: 'env:beta', stage: 'deploy', environment: 'beta', status: :running },
     { name: 'env:gamma', stage: 'deploy', environment: 'gamma', status: :canceled },
-    { name: 'staging', stage: 'deploy', environment: 'staging', status_event: :success },
-    { name: 'close staging', stage: 'deploy', environment: 'staging', when: 'manual', status: :skipped, options: { environment: { close: true } } },
+    { name: 'staging', stage: 'deploy', environment: 'staging', status_event: :success, options: { environment: { on_stop: 'stop staging' } } },
+    { name: 'stop staging', stage: 'deploy', environment: 'staging', when: 'manual', status: :skipped },
     { name: 'production', stage: 'deploy', environment: 'production', when: 'manual', status: :skipped },
     { name: 'slack', stage: 'notify', when: 'manual', status: :created },
   ]
diff --git a/db/migrate/20161006104309_add_state_to_environment.rb b/db/migrate/20161006104309_add_state_to_environment.rb
index 83dff07069f..ccb546654f9 100644
--- a/db/migrate/20161006104309_add_state_to_environment.rb
+++ b/db/migrate/20161006104309_add_state_to_environment.rb
@@ -6,7 +6,7 @@ class AddStateToEnvironment < ActiveRecord::Migration
   DOWNTIME = false
 
   def up
-    add_column_with_default(:environments, :state, :string, default: :opened)
+    add_column_with_default(:environments, :state, :string, default: :available)
   end
 
   def down
diff --git a/db/migrate/20161017095000_add_properties_to_deployment.rb b/db/migrate/20161017095000_add_properties_to_deployment.rb
new file mode 100644
index 00000000000..6371166a4d2
--- /dev/null
+++ b/db/migrate/20161017095000_add_properties_to_deployment.rb
@@ -0,0 +1,29 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddPropertiesToDeployment < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  # When using the methods "add_concurrent_index" or "add_column_with_default"
+  # you must disable the use of transactions as these methods can not run in an
+  # existing transaction. When using "add_concurrent_index" make sure that this
+  # method is the _only_ method called in the migration, any other changes
+  # should go in a separate migration. This ensures that upon failure _only_ the
+  # index creation fails and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  def change
+    add_column :deployments, :properties, :text
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index ba2cf686fa9..b574f4d4a6a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -398,22 +398,13 @@ ActiveRecord::Schema.define(version: 20161007133303) do
   add_index "emails", ["user_id"], name: "index_emails_on_user_id", using: :btree
 
   create_table "environments", force: :cascade do |t|
-<<<<<<< HEAD
-    t.integer  "project_id"
-    t.string   "name",                                null: false
-    t.datetime "created_at"
-    t.datetime "updated_at"
-    t.string   "external_url"
-    t.string   "environment_type"
-    t.string   "state",            default: "opened", null: false
-=======
     t.integer "project_id"
     t.string "name", null: false
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string "external_url"
     t.string "environment_type"
->>>>>>> origin/master
+    t.string "state", default: "available", null: false
   end
 
   add_index "environments", ["project_id", "name"], name: "index_environments_on_project_id_and_name", using: :btree