Merge branch 'time-tracking-integration' of gitlab.com:gitlab-org/gitlab-ee...

Merge branch 'time-tracking-integration' of gitlab.com:gitlab-org/gitlab-ee into time-tracking-integration

app/assets/javascripts/issuable_time_tracker.js.es6

@@ -140,11 +140,11 @@
                   <div :style='{ width: diffPercent }' class='meter-fill'></div>
                 </div>
                 <div class='compare-display-container'>
-                  <div class='compare-display pull-left'>
+                  <div class='compare-display'>
                     <span class='compare-label'>Spent</span>
                     <span class='compare-value spent'>{{ spentPretty }}</span>
                   </div>
-                  <div class='compare-display estimated pull-right'>
+                  <div class='compare-display estimated'>
                     <span class='compare-label'>Est</span>
                     <span class='compare-value'>{{ estimatedPretty }}</span>
                   </div>

app/assets/stylesheets/framework/variables.scss

@@ -5,7 +5,7 @@ $sidebar_collapsed_width: 62px;
 $sidebar_width: 220px;
 $gutter_collapsed_width: 62px;
 $gutter_width: 290px;
-$gutter_inner_width: 258px;
+$gutter_inner_width: 250px;
 $sidebar-transition-duration: .15s;
 $sidebar-breakpoint: 1024px;
 

app/assets/stylesheets/pages/issuable.scss

@@ -408,63 +408,80 @@
   }
 }
 
-#issuable-time-tracker {
+.time_tracker {
+  padding-bottom: 0;
+  border-bottom: 0;
+
   > .stopwatch-svg {
     display: none;
   }
 
-  .time-tracking-help-state {
-    padding: 10px 0;
-    margin-top: 10px;
-    border-top: 1px solid #dcdcdc;
-  }
-
-  .meter-container {
-    background: $gray-lighter;
-    border-radius: 2px;
-  }
-
-  .meter-fill {
-    max-width: 100%;
-    height: 4px;
-    background: $gl-text-green;
+  .sidebar-collapsed-icon {
+    svg {
+      width: 16px;
+      height: 16px;
+      fill: #999;
+    }
   }
 
   .help-button, .close-help-button {
     cursor: pointer;
   }
-  .over_estimate {
-    .meter-fill {
-      background: $red-light ;
-    }
-    .time-remaining, .compare-value.spent {
-      color: $red-light ;
+
+  .compare-meter {
+    &.within_estimate {
+      .meter-fill {
+        background: $gl-primary;
+      }
     }
-  }
 
-  .sidebar-collapsed-icon {
-    svg {
-      width: 16px;
-      height: 16px;
-      fill: #999;
+    &.over_estimate {
+      .meter-fill {
+        background: $red-light ;
+      }
+
+      .time-remaining, .compare-value.spent {
+        color: $red-light ;
+      }
     }
   }
 
-  .within_estimate {
+  .meter-container {
+    background: $border-gray-light;
+    border-radius: 3px;
+
     .meter-fill {
-      background: $gl-text-green;
+      max-width: 100%;
+      height: 5px;
+      border-radius: 3px;
+      background: $gl-primary;
     }
   }
+
   .compare-display-container {
-    height: 10px;
+    display: flex;
+    justify-content: space-between;
     margin-top: 5px;
-  }
-  .compare-display {
-    font-size: 13px;
-    color: $gl-gray-light;
 
-    .compare-value {
-      color: $gl-gray;
+    .compare-display {
+      font-size: 13px;
+      color: $gl-gray-light;
+
+      .compare-value {
+        color: $gl-gray;
+      }
     }
   }
+
+  .time-tracking-help-state {
+    background: $white-light;
+    margin: 16px -20px 0;
+    padding: 16px 20px;
+    border-top: 1px solid $border-gray-light;
+    border-bottom: 1px solid $border-gray-light;
+
+    a:hover {
+      color: $btn-white-active;
+    }
+  }  
 }

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
 

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

@@ -73,7 +73,7 @@
           = f.hidden_field 'milestone_id', value: issuable.milestone_id, id: nil
           = dropdown_tag('Milestone', options: { title: 'Assign milestone', toggle_class: 'js-milestone-select js-extra-options', filter: true, dropdown_class: 'dropdown-menu-selectable', placeholder: 'Search milestones', data: { show_no: true, field_name: "#{issuable.to_ability_name}[milestone_id]", project_id: @project.id, issuable_id: issuable.id, milestones: namespace_project_milestones_path(@project.namespace, @project, :json), ability_name: issuable.to_ability_name, issue_update: issuable_json_path(issuable), use_id: true }})
       - if issuable.has_attribute?(:time_estimate)
-        #issuable-time-tracker.block
+        #issuable-time-tracker.block.time_tracker
           %issuable-time-tracker{ ':time_estimate' => 'issuable.time_estimate', ':time_spent' => 'issuable.total_time_spent', ':human_time_estimate' => 'issuable.human_time_estimate', ':human_time_spent' => 'issuable.human_total_time_spent' }
             // Fallback while content is loading
             .title.hide-collapsed

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