Merge branch 'time-tracking-backend' into time-tracking-integration

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/notes_controller.rb

@@ -147,10 +147,9 @@ class Projects::NotesController < Projects::ApplicationController
 
   def note_json(note)
     if note.is_a?(AwardEmoji)
-      {
+      attrs = {
         valid:  note.valid?,
         award:  true,
-        id:     note.id,
         name:   note.name
       }
     elsif note.persisted?
@@ -158,10 +157,8 @@ class Projects::NotesController < Projects::ApplicationController
 
       attrs = {
         valid: true,
-        id: note.id,
         discussion_id: note.discussion_id,
         html: note_html(note),
-        award: false,
         note: note.note
       }
 
@@ -188,15 +185,18 @@ class Projects::NotesController < Projects::ApplicationController
           attrs[:original_discussion_id] = note.original_discussion_id
         end
       end
-
-      attrs
     else
-      {
+      attrs = {
         valid: false,
-        award: false,
         errors: note.errors
       }
     end
+
+    attrs[:award] ||= false
+    attrs[:id] = note.id
+    attrs[:commands_changes] = note.commands_changes
+
+    attrs
   end
 
   def authorize_admin_note!

app/models/concerns/issuable.rb

@@ -13,6 +13,7 @@ module Issuable
   include StripAttribute
   include Awardable
   include Taskable
+  include TimeTrackable
 
   included do
     cache_markdown_field :title, pipeline: :single_line
@@ -254,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
     {

app/models/concerns/time_trackable.rb

+# == TimeTrackable concern
+#
+# Contains functionality related to objects that support time tracking.
+#
+# Used by Issue and MergeRequest.
+#
+
+module TimeTrackable
+  extend ActiveSupport::Concern
+
+  included do
+    attr_reader :time_spent
+
+    alias_method :time_spent?, :time_spent
+
+    has_many :timelogs, as: :trackable, dependent: :destroy
+  end
+
+  def spend_time=(seconds:, user:)
+    # Exit if time to subtract exceeds the total time spent.
+    return if seconds < 0 && (seconds.abs > total_time_spent)
+
+    # When seconds = 0 we reset the total time spent by creating a new Timelog
+    # record with a negative value that is equal to the current total time spent.
+    new_time_spent = seconds.zero? ? (total_time_spent * -1) : seconds
+
+    timelogs.new(user: user, time_spent: new_time_spent)
+
+    @time_spent = seconds
+  end
+
+  def total_time_spent
+    timelogs.sum(:time_spent)
+  end
+end

app/models/note.rb

@@ -19,6 +19,9 @@ class Note < ActiveRecord::Base
   # Banzai::ObjectRenderer
   attr_accessor :user_visible_reference_count
 
+  # Attributes used to store the attributes that have ben changed by slash commands.
+  attr_accessor :commands_changes
+
   default_value_for :system, false
 
   attr_mentionable :note, pipeline: :note

app/models/timelog.rb

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

app/services/issuable_base_service.rb

@@ -36,6 +36,14 @@ class IssuableBaseService < BaseService
     end
   end
 
+  def create_time_estimate_note(issuable)
+    SystemNoteService.change_time_estimate(issuable, issuable.project, current_user)
+  end
+
+  def create_time_spent_note(issuable)
+    SystemNoteService.change_time_spent(issuable, issuable.project, current_user)
+  end
+
   def filter_params(issuable_ability_name = :issue)
     filter_assignee
     filter_milestone
@@ -249,6 +257,14 @@ class IssuableBaseService < BaseService
       create_task_status_note(issuable)
     end
 
+    if issuable.previous_changes.include?('time_estimate')
+      create_time_estimate_note(issuable)
+    end
+
+    if issuable.time_spent?
+      create_time_spent_note(issuable)
+    end
+
     create_labels_note(issuable, old_labels) if issuable.labels != old_labels
   end
 end

app/services/notes/create_service.rb

@@ -32,7 +32,7 @@ module Notes
         todo_service.new_note(note, current_user)
       end
 
-      if command_params && command_params.any?
+      if command_params.present?
         slash_commands_service.execute(command_params, note)
 
         # We must add the error after we call #save because errors are reset
@@ -40,6 +40,8 @@ module Notes
         if only_commands
           note.errors.add(:commands_only, 'Your commands have been executed!')
         end
+
+        note.commands_changes = command_params.keys
       end
 
       note

app/services/slash_commands/interpret_service.rb

@@ -248,6 +248,54 @@ module SlashCommands
     params '@user'
     command :cc
 
+    desc 'Set time estimate'
+    params '<1w 3d 2h 14m>'
+    condition do
+      current_user.can?(:"admin_#{issuable.to_ability_name}", project)
+    end
+    command :estimate do |raw_duration|
+      time_spent = ChronicDuration.parse(raw_duration, default_unit: 'hours') rescue nil
+
+      if time_spent
+        @updates[:time_estimate] = time_spent
+      end
+    end
+
+    desc 'Add or substract spent time'
+    params '<1h 30m | -1h 30m>'
+    condition do
+      current_user.can?(:"admin_#{issuable.to_ability_name}", issuable)
+    end
+    command :spend do |raw_duration|
+      reduce_time = raw_duration.sub!(/\A-/, '')
+      time_spent = ChronicDuration.parse(raw_duration, default_unit: 'hours') rescue nil
+
+      if time_spent
+        @updates[:spend_time] = {
+          seconds: reduce_time ? (time_spent * -1) : time_spent,
+          user: current_user
+        }
+      end
+    end
+
+    desc 'Remove the estimated time'
+    condition do
+      issuable.persisted? &&
+        current_user.can?(:"admin_#{issuable.to_ability_name}", project)
+    end
+    command :remove_estimation do
+      @updates[:time_estimate] = 0
+    end
+
+    desc 'Remove the time spent'
+    condition do
+      issuable.persisted? &&
+        current_user.can?(:"admin_#{issuable.to_ability_name}", project)
+    end
+    command :remove_time_spent do
+      @updates[:spend_time] = { seconds: 0, user: current_user }
+    end
+
     def find_label_ids(labels_param)
       label_ids_by_reference = extract_references(labels_param, :label).map(&:id)
       labels_ids_by_name = LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split).execute.select(:id)

app/services/system_note_service.rb

@@ -111,6 +111,57 @@ module SystemNoteService
     create_note(noteable: noteable, project: project, author: author, note: body)
   end
 
+  # Called when the estimated time of a Noteable is changed
+  #
+  # noteable      - Noteable object
+  # project       - Project owning noteable
+  # author        - User performing the change
+  # time_estimate - Estimated time
+  #
+  # Example Note text:
+  #
+  #   "Changed estimate of this issue to 3d 5h"
+  #
+  # Returns the created Note object
+
+  def change_time_estimate(noteable, project, author)
+    parsed_time = ChronicDuration.output(noteable.time_estimate, format: :short)
+    body = if parsed_time
+             "Changed time estimate of this #{noteable.human_class_name} to #{parsed_time}"
+           else
+             "Removed time estimate on this #{noteable.human_class_name}"
+           end
+
+    create_note(noteable: noteable, project: project, author: author, note: body)
+  end
+
+  # Called when the spent time of a Noteable is changed
+  #
+  # noteable   - Noteable object
+  # project    - Project owning noteable
+  # author     - User performing the change
+  # time_spent - Spent time
+  #
+  # Example Note text:
+  #
+  #   "Added 2h 30m of time spent on this issue"
+  #
+  # Returns the created Note object
+
+  def change_time_spent(noteable, project, author)
+    time_spent = noteable.time_spent
+
+    if time_spent.zero?
+      body = "Removed time spent on this #{noteable.human_class_name}"
+    else
+      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}"
+    end
+
+    create_note(noteable: noteable, project: project, author: author, note: body)
+  end
+
   # Called when the status of a Noteable is changed
   #
   # noteable - Noteable object

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'
 

config/initializers/chronic_duration.rb

 ChronicDuration.raise_exceptions = true
+
+# We may want to configure it through project settings in a future version.
+ChronicDuration.hours_per_day = 8
+ChronicDuration.days_per_week = 5

db/migrate/20161030005533_add_estimate_to_issuables.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddEstimateToIssuables < 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 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

db/migrate/20161030020610_create_timelogs.rb

+class CreateTimelogs < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    create_table :timelogs do |t|
+      t.integer :time_spent, null: false
+      t.references :trackable, polymorphic: true
+      t.references :user
+
+      t.timestamps null: false
+    end
+
+    add_index :timelogs, [:trackable_type, :trackable_id]
+    add_index :timelogs, :user_id
+  end
+end

db/schema.rb

@@ -560,6 +560,7 @@ ActiveRecord::Schema.define(version: 20161106185620) do
     t.integer "lock_version"
     t.text "title_html"
     t.text "description_html"
+    t.integer "time_estimate", default: 0
   end
 
   add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
@@ -756,6 +757,7 @@ ActiveRecord::Schema.define(version: 20161106185620) do
     t.integer "lock_version"
     t.text "title_html"
     t.text "description_html"
+    t.integer "time_estimate", default: 0
   end
 
   add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
@@ -1251,6 +1253,18 @@ ActiveRecord::Schema.define(version: 20161106185620) do
 
   add_index "tags", ["name"], name: "index_tags_on_name", unique: true, using: :btree
 
+  create_table "timelogs", force: :cascade do |t|
+    t.integer "time_spent", null: false
+    t.integer "trackable_id"
+    t.string "trackable_type"
+    t.integer "user_id"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+  end
+
+  add_index "timelogs", ["trackable_type", "trackable_id"], name: "index_timelogs_on_trackable_type_and_trackable_id", using: :btree
+  add_index "timelogs", ["user_id"], name: "index_timelogs_on_user_id", using: :btree
+
   create_table "todos", force: :cascade do |t|
     t.integer "user_id", null: false
     t.integer "project_id", null: false

doc/user/project/slash_commands.md

@@ -29,3 +29,7 @@ do.
 | <code>/due &lt;in 2 days &#124; this Friday &#124; December 31st&gt;</code> | Set due date |
 | `/remove_due_date`         | Remove due date |
 | `/wip`                     | Toggle the Work In Progress status |
+| <code>/estimate &lt;1w 3d 2h 14m&gt;</code> | Set time estimate |
+| `/remove_estimation`       | Remove estimated time |
+| <code>/spend &lt;1h 30m &#124; -1h 5m&gt;</code> | Add or substract spent time |
+| `/remove_time_spent`       | Remove time spent |

lib/gitlab/import_export/import_export.yml

@@ -6,6 +6,7 @@ project_tree:
     - :events
   - issues:
     - :events
+    - :timelogs
     - notes:
       - :author
       - :events
@@ -27,6 +28,7 @@ project_tree:
       - :events
     - :merge_request_diff
     - :events
+    - :timelogs
     - label_links:
       - label:
           :priorities

spec/factories/timelogs.rb

+# Read about factories at https://github.com/thoughtbot/factory_girl
+
+FactoryGirl.define do
+  factory :timelog do
+    time_spent 3600
+    association :trackable, factory: :issue
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -15,6 +15,7 @@ issues:
 - events
 - merge_requests_closing_issues
 - metrics
+- timelogs
 events:
 - author
 - project
@@ -80,6 +81,7 @@ merge_requests:
 - approvals
 - approvers
 - approver_groups
+- timelogs
 merge_request_diff:
 - merge_request
 pipelines:
@@ -207,3 +209,5 @@ award_emoji:
 - user
 priorities:
 - label
+timelogs:
+- trackable

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -20,6 +20,7 @@ Issue:
 - lock_version
 - milestone_id
 - weight
+- time_estimate
 Event:
 - id
 - target_type
@@ -151,6 +152,7 @@ MergeRequest:
 - milestone_id
 - approvals_before_merge
 - rebase_commit_sha
+- time_estimate
 MergeRequestDiff:
 - id
 - state
@@ -346,3 +348,11 @@ LabelPriority:
 - priority
 - created_at
 - updated_at
+Timelog:
+- id
+- time_spent
+- trackable_id
+- trackable_type
+- user_id
+- created_at
+- updated_at

spec/models/concerns/issuable_spec.rb

@@ -384,4 +384,37 @@ describe Issue, "Issuable" do
       expect(issue.assignee_or_author?(user)).to eq(false)
     end
   end
+
+  describe '#spend_time' do
+    let(:user) { create(:user) }
+    let(:issue) { create(:issue) }
+
+    context 'adding time' do
+      it 'should update the total time spent' do
+        issue.update_attributes!(spend_time: { seconds: 1800, user: user })
+
+        expect(issue.total_time_spent).to eq(1800)
+      end
+    end
+
+    context 'substracting time' do
+      before do
+        issue.update_attributes!(spend_time: { seconds: 1800, user: user })
+      end
+
+      it 'should update the total time spent' do
+        issue.update_attributes!(spend_time: { seconds: -900, user: user })
+
+        expect(issue.total_time_spent).to eq(900)
+      end
+
+      context 'when time to substract exceeds the total time spent' do
+        it 'should not alter the total time spent' do
+          issue.update_attributes!(spend_time: { seconds: -3600, user: user })
+
+          expect(issue.total_time_spent).to eq(1800)
+        end
+      end
+    end
+  end
 end

spec/models/timelog_spec.rb

+require 'rails_helper'
+
+RSpec.describe Timelog, type: :model do
+  subject { build(:timelog) }
+
+  it { is_expected.to be_valid }
+
+  it { is_expected.to validate_presence_of(:time_spent) }
+end

spec/services/notes/slash_commands_service_spec.rb

@@ -84,6 +84,18 @@ describe Notes::SlashCommandsService, services: true do
           expect(note.noteable).to be_open
         end
       end
+
+      describe '/spend' do
+        let(:note_text) { '/spend 1h' }
+
+        it 'updates the spent time on the noteable' do
+          content, command_params = service.extract_commands(note)
+          service.execute(command_params, note)
+
+          expect(content).to eq ''
+          expect(note.noteable.time_spent).to eq(3600)
+        end
+      end
     end
 
     describe 'note with command & text' do

spec/services/slash_commands/interpret_service_spec.rb

@@ -210,6 +210,46 @@ describe SlashCommands::InterpretService, services: true do
       end
     end
 
+    shared_examples 'estimate command' do
+      it 'populates time_estimate: "3600" if content contains /estimate 1h' do
+        _, updates = service.execute(content, issuable)
+
+        expect(updates).to eq(time_estimate: 3600)
+      end
+    end
+
+    shared_examples 'spend command' do
+      it 'populates spend_time: { seconds: 3600, user: user } if content contains /spend 1h' do
+        _, updates = service.execute(content, issuable)
+
+        expect(updates).to eq(spend_time: { seconds: 3600, user: developer })
+      end
+    end
+
+    shared_examples 'spend command with negative time' do
+      it 'populates spend_time: { seconds: -1800, user: user } if content contains /spend -30m' do
+        _, updates = service.execute(content, issuable)
+
+        expect(updates).to eq(spend_time: { seconds: -1800, user: developer })
+      end
+    end
+
+    shared_examples 'remove_estimation command' do
+      it 'populates time_estimate: "0" if content contains /remove_estimation' do
+        _, updates = service.execute(content, issuable)
+
+        expect(updates).to eq(time_estimate: 0)
+      end
+    end
+
+    shared_examples 'remove_time_spent command' do
+      it 'populates spend_time: "0" if content contains /remove_time_spent' do
+        _, updates = service.execute(content, issuable)
+
+        expect(updates).to eq(spend_time: { seconds: 0, user: developer })
+      end
+    end
+
     shared_examples 'empty command' do
       it 'populates {} if content contains an unsupported command' do
         _, updates = service.execute(content, issuable)
@@ -451,6 +491,51 @@ describe SlashCommands::InterpretService, services: true do
       let(:issuable) { merge_request }
     end
 
+    it_behaves_like 'estimate command' do
+      let(:content) { '/estimate 1h' }
+      let(:issuable) { issue }
+    end
+
+    it_behaves_like 'empty command' do
+      let(:content) { '/estimate' }
+      let(:issuable) { issue }
+    end
+
+    it_behaves_like 'empty command' do
+      let(:content) { '/estimate abc' }
+      let(:issuable) { issue }
+    end
+
+    it_behaves_like 'spend command' do
+      let(:content) { '/spend 1h' }
+      let(:issuable) { issue }
+    end
+
+    it_behaves_like 'spend command with negative time' do
+      let(:content) { '/spend -30m' }
+      let(:issuable) { issue }
+    end
+
+    it_behaves_like 'empty command' do
+      let(:content) { '/spend' }
+      let(:issuable) { issue }
+    end
+
+    it_behaves_like 'empty command' do
+      let(:content) { '/spend abc' }
+      let(:issuable) { issue }
+    end
+
+    it_behaves_like 'remove_estimation command' do
+      let(:content) { '/remove_estimation' }
+      let(:issuable) { issue }
+    end
+
+    it_behaves_like 'remove_time_spent command' do
+      let(:content) { '/remove_time_spent' }
+      let(:issuable) { issue }
+    end
+
     context 'when current_user cannot :admin_issue' do
       let(:visitor) { create(:user) }
       let(:issue) { create(:issue, project: project, author: visitor) }