Address most of the feedback from last code review.

app/controllers/concerns/issuable_actions.rb

@@ -12,7 +12,7 @@ module IssuableActions
     destroy_method = "destroy_#{issuable.class.name.underscore}".to_sym
     TodoService.new.public_send(destroy_method, issuable, current_user)
 
-    name = issuable.class.name.titleize.downcase
+    name = issuable.human_class_name
     flash[:notice] = "The #{name} was successfully deleted."
     redirect_to polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable.class])
   end

app/controllers/projects/issues_controller.rb

@@ -221,8 +221,7 @@ class Projects::IssuesController < Projects::ApplicationController
   def issue_params
     params.require(:issue).permit(
       :title, :assignee_id, :position, :description, :confidential, :weight,
-      :milestone_id, :due_date, :state_event, :task_num, :lock_version, :time_estimate,
-      label_ids: [],
+      :milestone_id, :due_date, :state_event, :task_num, :lock_version, label_ids: []
     )
   end
 end

app/controllers/projects/merge_requests_controller.rb

@@ -674,7 +674,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       approvals_before_merge
       approver_group_ids
       approver_ids
-      time_estimate
     ]
   end
 

app/models/concerns/issuable.rb

@@ -255,6 +255,17 @@ module Issuable
     self.class.to_ability_name
   end
 
+  # Convert this Issuable class name to a format usable by notifications.
+  #
+  # Examples:
+  #
+  #   issuable.class           # => MergeRequest
+  #   issuable.human_class_name # => "merge request"
+
+  def human_class_name
+    @human_class_name ||= self.class.name.titleize.downcase
+  end
+
   # Returns a Hash of attributes to be used for Twitter card metadata
   def card_attributes
     {
@@ -307,4 +318,8 @@ module Issuable
   def time_spent
     timelogs.sum(:time_spent)
   end
+
+  def spend_time(seconds)
+    timelogs.new(time_spent: seconds)
+  end
 end

app/models/timelog.rb

 class Timelog < ActiveRecord::Base
-  validates :time_spent, :trackable, presence: true
+  validates :time_spent, presence: true
 
   belongs_to :trackable, polymorphic: true
 end

app/services/issuable_base_service.rb

@@ -37,8 +37,7 @@ class IssuableBaseService < BaseService
   end
 
   def create_time_estimate_note(issuable)
-    SystemNoteService.change_time_estimate(
-      issuable, issuable.project, current_user, issuable.time_estimate)
+    SystemNoteService.change_time_estimate(issuable, issuable.project, current_user)
   end
 
   def create_time_spent_note(issuable, time_spent)
@@ -147,6 +146,7 @@ class IssuableBaseService < BaseService
   def create(issuable)
     merge_slash_commands_into_params!(issuable)
     filter_params
+    change_spent_time(issuable)
 
     params.delete(:state_event)
     params[:author] ||= current_user
@@ -241,7 +241,7 @@ class IssuableBaseService < BaseService
 
   def change_spent_time(issuable)
     if time_spent
-      issuable.timelogs.new(time_spent: time_spent)
+      issuable.spend_time(time_spent)
     end
   end
 

app/services/slash_commands/interpret_service.rb

@@ -248,25 +248,24 @@ module SlashCommands
     params '@user'
     command :cc
 
-    desc 'Set estimate'
-    params 'e.g: 1w 3d 2h 14m'
+    desc 'Set time estimate'
+    params '<1w 3d 2h 14m>'
     condition do
-      current_user.can?(:"update_#{issuable.to_ability_name}", project)
+      current_user.can?(:"admin_#{issuable.to_ability_name}", project)
     end
     command :estimate do |raw_duration|
       @updates[:time_estimate] = ChronicDuration.parse(raw_duration, default_unit: 'hours')
     end
 
     desc 'Enter current spent time'
-    params 'e.g: 3h 30m'
+    params '<1h 30m>'
     condition do
-      issuable.persisted? &&
-        current_user.can?(:"update_#{issuable.to_ability_name}", issuable)
+      current_user.can?(:"admin_#{issuable.to_ability_name}", issuable)
     end
     command :spend do |raw_duration|
-      reduce_time = raw_duration.gsub!(/\A-/, '')
+      reduce_time = raw_duration.sub!(/\A-/, '')
       time_spent  = ChronicDuration.parse(raw_duration, default_unit: 'hours')
-      time_spent  = time_spent * -1 if reduce_time
+      time_spent  *= -1 if reduce_time
 
       @updates[:time_spent] = time_spent
     end

app/services/system_note_service.rb

@@ -124,9 +124,9 @@ module SystemNoteService
   #
   # Returns the created Note object
 
-  def change_time_estimate(noteable, project, author, time_estimate)
-    parsed_time = ChronicDuration.output(time_estimate, format: :short)
-    body = "Changed estimate of this #{noteable.to_ability_name} to #{parsed_time}"
+  def change_time_estimate(noteable, project, author)
+    parsed_time = ChronicDuration.output(noteable.time_estimate, format: :short)
+    body = "Changed time estimate of this #{noteable.human_class_name} to #{parsed_time}"
 
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
@@ -145,8 +145,9 @@ module SystemNoteService
   # Returns the created Note object
 
   def change_time_spent(noteable, project, author, time_spent)
-    parsed_time = ChronicDuration.output(time_spent, format: :short)
-    body = "Added #{parsed_time} of time spent on this #{noteable.to_ability_name}"
+    parsed_time = ChronicDuration.output(time_spent.abs, format: :short)
+    action = time_spent > 0 ? 'Added' : 'Substracted'
+    body = "#{action} #{parsed_time} of time spent on this #{noteable.human_class_name}"
 
     create_note(noteable: noteable, project: project, author: author, note: body)
   end

app/views/shared/issuable/_form.html.haml

@@ -173,7 +173,7 @@
   - else
     .pull-right
       - if can?(current_user, :"destroy_#{issuable.to_ability_name}", @project)
-        = link_to 'Delete', polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), data: { confirm: "#{issuable.class.name.titleize} will be removed! Are you sure?" },
+        = link_to 'Delete', polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), data: { confirm: "#{issuable.human_class_name} will be removed! Are you sure?" },
                                                                                                   method: :delete, class: 'btn btn-danger btn-grouped'
       = link_to 'Cancel', polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), class: 'btn btn-grouped btn-cancel'
 

db/migrate/20161030005533_add_estimate_to_issuables.rb

@@ -23,8 +23,13 @@ class AddEstimateToIssuables < ActiveRecord::Migration
   # comments:
   # disable_ddl_transaction!
 
-  def change
+  def up
     add_column_with_default :issues, :time_estimate, :integer, default: 0
     add_column_with_default :merge_requests, :time_estimate, :integer, default: 0
   end
+
+  def down
+    remove_column :issues, :time_estimate
+    remove_column :merge_requests, :time_estimate
+  end
 end