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

Conflicts:
	app/controllers/projects/issues_controller.rb
	app/models/concerns/time_trackable.rb
	db/schema.rb

app/controllers/projects/issues_controller.rb

@@ -75,7 +75,7 @@ class Projects::IssuesController < Projects::ApplicationController
     respond_to do |format|
       format.html
       format.json do
-        render json: @issue.to_json(include: [:milestone, :labels], methods: [:total_time_spent, :human_total_time_spent, :human_time_estimate])
+        render json: IssueSerializer.new.represent(@issue)
       end
     end
   end

app/controllers/projects/merge_requests_controller.rb

@@ -62,7 +62,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       format.html { define_discussion_vars }
 
       format.json do
-        render json: @merge_request, methods: :rebase_in_progress?
+        render json: MergeRequestSerializer.new.represent(@merge_request)
       end
 
       format.patch  do

app/controllers/projects/notes_controller.rb

@@ -148,8 +148,7 @@ class Projects::NotesController < Projects::ApplicationController
   def note_json(note)
     attrs = {
       award: false,
-      id: note.id,
-      commands_changes: note.commands_changes
+      id: note.id
     }
 
     if note.is_a?(AwardEmoji)
@@ -198,6 +197,7 @@ class Projects::NotesController < Projects::ApplicationController
       )
     end
 
+    attrs[:commands_changes] = note.commands_changes unless attrs[:award]
     attrs
   end
 

app/models/concerns/time_trackable.rb

@@ -27,25 +27,20 @@ module TimeTrackable
     if seconds == :reset
       reset_spent_time
     else
-      add_or_susbtract_spent_time
+      add_or_subtract_spent_time
     end
   end
 
-  def spend_time!(seconds, user)
-    spend_time(seconds, user)
-    save!
-  end
-
   def total_time_spent
     timelogs.sum(:time_spent)
   end
 
   def human_total_time_spent
-    ChronicDuration.output(total_time_spent, format: :short)
+    Gitlab::TimeTrackingFormatter.output(total_time_spent)
   end
 
   def human_time_estimate
-    ChronicDuration.output(time_estimate, format: :short)
+    Gitlab::TimeTrackingFormatter.output(time_estimate)
   end
 
   private
@@ -54,7 +49,7 @@ module TimeTrackable
     timelogs.new(time_spent: total_time_spent * -1, user: @time_spent_user)
   end
 
-  def add_or_susbtract_spent_time
+  def add_or_subtract_spent_time
     # Exit if time to subtract exceeds the total time spent.
     return if time_spent < 0 && (time_spent.abs > total_time_spent)
 

app/models/note.rb

@@ -20,7 +20,7 @@ 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.
+  # Attribute used to store the attributes that have ben changed by slash commands.
   attr_accessor :commands_changes
 
   default_value_for :system, false

app/models/timelog.rb

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

app/serializers/issuable_entity.rb

+class IssuableEntity < Grape::Entity
+  expose :id
+  expose :iid
+  expose :assignee_id
+  expose :author_id
+  expose :description
+  expose :lock_version
+  expose :milestone_id
+  expose :position
+  expose :state
+  expose :title
+  expose :updated_by_id
+  expose :created_at
+  expose :updated_at
+  expose :deleted_at
+  expose :time_estimate
+  expose :total_time_spent
+  expose :human_time_estimate
+  expose :human_total_time_spent
+end

app/serializers/issue_entity.rb

+class IssueEntity < IssuableEntity
+  expose :branch_name
+  expose :confidential
+  expose :due_date
+  expose :moved_to_id
+  expose :project_id
+  expose :updated_by_id
+  expose :weight
+  expose :milestone, using: API::Entities::Milestone
+  expose :labels, using: LabelEntity
+end

app/serializers/issue_serializer.rb

+class IssueSerializer < BaseSerializer
+  entity IssueEntity
+end

app/serializers/label_entity.rb

+class LabelEntity < Grape::Entity
+  expose :id
+  expose :title
+  expose :color
+  expose :description
+  expose :group_id
+  expose :project_id
+  expose :template
+  expose :created_at
+  expose :updated_at
+end

app/serializers/merge_request_entity.rb

+class MergeRequestEntity < IssuableEntity
+  expose :approvals_before_merge
+  expose :in_progress_merge_commit_sha
+  expose :locked_at
+  expose :merge_commit_sha
+  expose :merge_error
+  expose :merge_params
+  expose :merge_status
+  expose :merge_user_id
+  expose :merge_when_build_succeeds
+  expose :rebase_commit_sha
+  expose :rebase_in_progress?
+  expose :source_branch
+  expose :source_project_id
+  expose :target_branch
+  expose :target_project_id
+end

app/serializers/merge_request_serializer.rb

+class MergeRequestSerializer < BaseSerializer
+  entity MergeRequestEntity
+end

app/services/issuable_base_service.rb

@@ -239,9 +239,9 @@ class IssuableBaseService < BaseService
   end
 
   def change_time_spent(issuable)
-    if params[:spend_time]
-      issuable.spend_time(params.delete(:spend_time), current_user)
-    end
+    time_spent = params.delete(:spend_time)
+
+    issuable.spend_time(time_spent, current_user) if time_spent
   end
 
   def has_changes?(issuable, old_labels: [])

app/services/slash_commands/interpret_service.rb

@@ -254,7 +254,7 @@ module SlashCommands
       current_user.can?(:"admin_#{issuable.to_ability_name}", project)
     end
     command :estimate do |raw_duration|
-      time_estimate = ChronicDuration.parse(raw_duration, default_unit: 'hours') rescue nil
+      time_estimate = Gitlab::TimeTrackingFormatter.parse(raw_duration)
 
       if time_estimate
         @updates[:time_estimate] = time_estimate
@@ -268,10 +268,11 @@ module SlashCommands
     end
     command :spend do |raw_duration|
       reduce_time = raw_duration.sub!(/\A-/, '')
-      time_spent = ChronicDuration.parse(raw_duration, default_unit: 'hours') rescue nil
-      time_spent *= -1 if time_spent && reduce_time
+      time_spent = Gitlab::TimeTrackingFormatter.parse(raw_duration)
 
       if time_spent
+        time_spent *= -1 if reduce_time
+
         @updates[:spend_time] = time_spent
       end
     end

app/services/system_note_service.rb

@@ -125,11 +125,11 @@ module SystemNoteService
   # 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
+    parsed_time = Gitlab::TimeTrackingFormatter.output(noteable.time_estimate)
+    body = if noteable.time_estimate == 0
              "Removed time estimate on this #{noteable.human_class_name}"
+           else
+             "Changed time estimate of this #{noteable.human_class_name} to #{parsed_time}"
            end
 
     create_note(noteable: noteable, project: project, author: author, note: body)
@@ -154,8 +154,8 @@ module SystemNoteService
     if time_spent == :reset
       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'
+      parsed_time = Gitlab::TimeTrackingFormatter.output(time_spent.abs)
+      action = time_spent > 0 ? 'Added' : 'Subtracted'
       body = "#{action} #{parsed_time} of time spent on this #{noteable.human_class_name}"
     end
 

db/schema.rb

@@ -825,9 +825,9 @@ ActiveRecord::Schema.define(version: 20161109150329) do
     t.datetime "ldap_sync_last_successful_update_at"
     t.datetime "ldap_sync_last_sync_at"
     t.datetime "deleted_at"
+    t.text "description_html"
     t.boolean "lfs_enabled"
     t.integer "repository_size_limit"
-    t.text "description_html"
   end
 
   add_index "namespaces", ["created_at"], name: "index_namespaces_on_created_at", using: :btree
@@ -1010,7 +1010,6 @@ ActiveRecord::Schema.define(version: 20161109150329) do
     t.datetime "created_at"
     t.datetime "updated_at"
     t.integer "creator_id"
-    t.boolean "wall_enabled", default: true, null: false
     t.integer "namespace_id"
     t.datetime "last_activity_at"
     t.string "import_url"

doc/workflow/README.md

@@ -23,6 +23,7 @@
 - [Slash commands](../user/project/slash_commands.md)
 - [Sharing a project with a group](share_with_group.md)
 - [Share projects with other groups](share_projects_with_other_groups.md)
+- [Time tracking](time_tracking.md)
 - [Two-factor Authentication (2FA)](two_factor_authentication.md)
 - [Web Editor](../user/project/repository/web_editor.md)
 - [Releases](releases.md)

doc/workflow/time_tracking.md

+# Time Tracking
+
+> Introduced in GitLab 8.14.
+
+Time Tracking lets teams stack their project estimates against their time spent.
+
+Other interesting links:
+
+- [Time Tracking landing page on about.gitlab.com][landing]
+
+## Overview
+
+Time Tracking lets you
+* record the time spent working on an issue or a merge request,
+* add an estimate of the amount of time needed to complete an issue or a merge
+request.
+
+You don't have to indicate an estimate to enter the time spent, and vice versa.
+
+Data about time tracking is shown on the issue/merge request sidebar, as shown
+below.
+
+![Time tracking in the sidebar](time-tracking/time-tracking-sidebar.png)
+
+## How to enter data
+
+Time Tracking uses two [slash commands] that GitLab introduced with this new
+feature: `/spend` and `/estimate`.
+
+Slash commands can be used in the body of an issue or a merge request, but also
+in a comment in both an issue or a merge request.
+
+Below is an example of how you can use those new slash commands inside a comment.
+
+![Time tracking example in a comment](time-tracking/time-tracking-example.png)
+
+Adding time entries (time spent or estimates) is limited to project members.
+
+### Estimates
+
+To enter an estimate, write `/estimate`, followed by the time. For example, if
+you need to enter an estimate of 3 days, 5 hours and 10 minutes, you would write
+`/estimate 3d 5h 10m`.
+
+Every time you enter a new time estimate, any previous time estimates will be
+overridden by this new value. There should only be one valid estimate in an
+issue or a merge request.
+
+To remove an estimation entirely, use `/remove_estimation`.
+
+### Time spent
+
+To enter a time spent, use `/spend 3d 5h 10m`.
+
+Every new time spent entry will be added to the current total time spent for the
+issue or the merge request.
+
+You can remove time by entering a negative amount: `/spend -3d` will remove 3
+days from the total time spent. You can't go below 0 minutes of time spent,
+so GitLab will automatically reset the time spent if you remove a larger amount
+of time compared to the time that was entered already.
+
+To remove all the time spent at once, use `/remove_time_spent`.
+
+## Configuration
+
+The following time units are available:
+* weeks (w)
+* days (d)
+* hours (h)
+* minutes (m)
+
+Default conversion rates are 1w = 5d and 1d = 8h.
+
+[landing]: https://about.gitlab.com/features/time-tracking
+[slash-commands]: ../user/project/slash_commands.md

lib/gitlab/time_tracking_formatter.rb

+module Gitlab
+  module TimeTrackingFormatter
+    extend self
+
+    def parse(string)
+      ChronicDuration.parse(string, default_unit: 'hours')
+    rescue ChronicDuration::DurationParseError
+      nil
+    end
+
+    def output(seconds)
+      ChronicDuration.output(seconds, format: :short, limit_to_hours: false, weeks: true)
+    end
+  end
+end

spec/factories/timelogs.rb

@@ -3,6 +3,7 @@
 FactoryGirl.define do
   factory :timelog do
     time_spent 3600
+    user
     association :trackable, factory: :issue
   end
 end

spec/lib/gitlab/import_export/all_models.yml

@@ -211,3 +211,4 @@ priorities:
 - label
 timelogs:
 - trackable
+- user

spec/models/concerns/issuable_spec.rb

@@ -389,9 +389,14 @@ describe Issue, "Issuable" do
     let(:user) { create(:user) }
     let(:issue) { create(:issue) }
 
+    def spend_time(seconds)
+      issue.spend_time(seconds, user)
+      issue.save!
+    end
+
     context 'adding time' do
       it 'should update the total time spent' do
-        issue.spend_time!(1800, user)
+        spend_time(1800)
 
         expect(issue.total_time_spent).to eq(1800)
       end
@@ -399,18 +404,18 @@ describe Issue, "Issuable" do
 
     context 'substracting time' do
       before do
-        issue.spend_time!(1800, user)
+        spend_time(1800)
       end
 
       it 'should update the total time spent' do
-        issue.spend_time!(-900, user)
+        spend_time(-900)
 
         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.spend_time!(-3600, user)
+          spend_time(-3600)
 
           expect(issue.total_time_spent).to eq(1800)
         end

spec/models/timelog_spec.rb

@@ -6,4 +6,5 @@ RSpec.describe Timelog, type: :model do
   it { is_expected.to be_valid }
 
   it { is_expected.to validate_presence_of(:time_spent) }
+  it { is_expected.to validate_presence_of(:user) }
 end

spec/services/slash_commands/interpret_service_spec.rb

@@ -211,7 +211,7 @@ describe SlashCommands::InterpretService, services: true do
     end
 
     shared_examples 'estimate command' do
-      it 'populates time_estimate: "3600" if content contains /estimate 1h' do
+      it 'populates time_estimate: 3600 if content contains /estimate 1h' do
         _, updates = service.execute(content, issuable)
 
         expect(updates).to eq(time_estimate: 3600)
@@ -219,7 +219,7 @@ describe SlashCommands::InterpretService, services: true do
     end
 
     shared_examples 'spend command' do
-      it 'populates spend_time: { seconds: 3600, user: user } if content contains /spend 1h' do
+      it 'populates spend_time: 3600 if content contains /spend 1h' do
         _, updates = service.execute(content, issuable)
 
         expect(updates).to eq(spend_time: 3600)
@@ -227,7 +227,7 @@ describe SlashCommands::InterpretService, services: true do
     end
 
     shared_examples 'spend command with negative time' do
-      it 'populates spend_time: { seconds: -1800, user: user } if content contains /spend -30m' do
+      it 'populates spend_time: -1800 if content contains /spend -30m' do
         _, updates = service.execute(content, issuable)
 
         expect(updates).to eq(spend_time: -1800)
@@ -235,7 +235,7 @@ describe SlashCommands::InterpretService, services: true do
     end
 
     shared_examples 'remove_estimate command' do
-      it 'populates time_estimate: "0" if content contains /remove_estimate' do
+      it 'populates time_estimate: 0 if content contains /remove_estimate' do
         _, updates = service.execute(content, issuable)
 
         expect(updates).to eq(time_estimate: 0)
@@ -243,7 +243,7 @@ describe SlashCommands::InterpretService, services: true do
     end
 
     shared_examples 'remove_time_spent command' do
-      it 'populates spend_time: ":reset" if content contains /remove_time_spent' do
+      it 'populates spend_time: :reset if content contains /remove_time_spent' do
         _, updates = service.execute(content, issuable)
 
         expect(updates).to eq(spend_time: :reset)

spec/services/system_note_service_spec.rb

@@ -594,4 +594,69 @@ describe SystemNoteService, services: true do
       end
     end
   end
+
+  describe '.change_time_estimate' do
+    subject { described_class.change_time_estimate(noteable, project, author) }
+
+    it_behaves_like 'a system note'
+
+    context 'with a time estimate' do
+      it 'sets the note text' do
+        noteable.update_attribute(:time_estimate, 277200)
+
+        expect(subject.note).to eq "Changed time estimate of this issue to 1w 4d 5h"
+      end
+    end
+
+    context 'without a time estimate' do
+      it 'sets the note text' do
+        expect(subject.note).to eq "Removed time estimate on this issue"
+      end
+    end
+  end
+
+  describe '.change_time_spent' do
+    # We need a custom noteable in order to the shared examples to be green.
+    let(:noteable) do
+      mr = create(:merge_request, source_project: project)
+      mr.spend_time(1, author)
+      mr.save!
+      mr
+    end
+
+    subject do
+      described_class.change_time_spent(noteable, project, author)
+    end
+
+    it_behaves_like 'a system note'
+
+    context 'when time was added' do
+      it 'sets the note text' do
+        spend_time!(277200)
+
+        expect(subject.note).to eq "Added 1w 4d 5h of time spent on this merge request"
+      end
+    end
+
+    context 'when time was subtracted' do
+      it 'sets the note text' do
+        spend_time!(-277200)
+
+        expect(subject.note).to eq "Subtracted 1w 4d 5h of time spent on this merge request"
+      end
+    end
+
+    context 'when time was removed' do
+      it 'sets the note text' do
+        spend_time!(:reset)
+
+        expect(subject.note).to eq "Removed time spent on this merge request"
+      end
+    end
+
+    def spend_time!(seconds)
+      noteable.spend_time(seconds, author)
+      noteable.save!
+    end
+  end
 end