From 4e283ee706b11fd3b918230976addc9a70603ce6 Mon Sep 17 00:00:00 2001 From: Jon Kolb Date: Tue, 11 Jun 2019 17:48:42 +0800 Subject: Limit time tracking values to hours Adds an instance setting to limit display of time tracking values to hours only --- .../boards/components/issue_time_estimate.vue | 9 +++++++-- .../javascripts/lib/utils/datetime_utility.js | 9 ++++++++- .../components/time_tracking/comparison_pane.vue | 2 +- app/helpers/application_settings_helper.rb | 1 + app/models/application_setting_implementation.rb | 1 + .../application_settings/_localization.html.haml | 6 ++++++ .../30355-use-hours-only-for-time-tracking.yml | 5 +++++ ...ng_display_hours_only_to_application_settings.rb | 21 +++++++++++++++++++++ db/schema.rb | 1 + doc/api/settings.md | 1 + lib/gitlab/gon_helper.rb | 2 ++ lib/gitlab/time_tracking_formatter.rb | 3 ++- 12 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/30355-use-hours-only-for-time-tracking.yml create mode 100644 db/migrate/20190611090827_add_time_tracking_display_hours_only_to_application_settings.rb diff --git a/app/assets/javascripts/boards/components/issue_time_estimate.vue b/app/assets/javascripts/boards/components/issue_time_estimate.vue index 98c1d29db16..2545980163f 100644 --- a/app/assets/javascripts/boards/components/issue_time_estimate.vue +++ b/app/assets/javascripts/boards/components/issue_time_estimate.vue @@ -16,10 +16,15 @@ export default { }, computed: { title() { - return stringifyTime(parseSeconds(this.estimate), true); + return stringifyTime( + parseSeconds(this.estimate, { limitToHours: gon.time_tracking_display_hours_only }), + true + ); }, timeEstimate() { - return stringifyTime(parseSeconds(this.estimate)); + return stringifyTime( + parseSeconds(this.estimate, { limitToHours: gon.time_tracking_display_hours_only }) + ); }, }, }; diff --git a/app/assets/javascripts/lib/utils/datetime_utility.js b/app/assets/javascripts/lib/utils/datetime_utility.js index d521c462ad8..f8398f2f780 100644 --- a/app/assets/javascripts/lib/utils/datetime_utility.js +++ b/app/assets/javascripts/lib/utils/datetime_utility.js @@ -479,7 +479,10 @@ export const pikadayToString = date => { * Seconds can be negative or positive, zero or non-zero. Can be configured for any day * or week length. */ -export const parseSeconds = (seconds, { daysPerWeek = 5, hoursPerDay = 8 } = {}) => { +export const parseSeconds = ( + seconds, + { daysPerWeek = 5, hoursPerDay = 8, limitToHours = false } = {}, +) => { const DAYS_PER_WEEK = daysPerWeek; const HOURS_PER_DAY = hoursPerDay; const MINUTES_PER_HOUR = 60; @@ -496,6 +499,10 @@ export const parseSeconds = (seconds, { daysPerWeek = 5, hoursPerDay = 8 } = {}) let unorderedMinutes = Math.abs(seconds / MINUTES_PER_HOUR); return _.mapObject(timePeriodConstraints, minutesPerPeriod => { + if (limitToHours && minutesPerPeriod > MINUTES_PER_HOUR) { + return 0; + } + const periodCount = Math.floor(unorderedMinutes / minutesPerPeriod); unorderedMinutes -= periodCount * minutesPerPeriod; diff --git a/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue b/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue index f4d926cd3ec..cd3e7ce33f7 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue @@ -32,7 +32,7 @@ export default { computed: { parsedTimeRemaining() { const diffSeconds = this.timeEstimate - this.timeSpent; - return parseSeconds(diffSeconds); + return parseSeconds(diffSeconds, { limitToHours: gon.time_tracking_display_hours_only }); }, timeRemainingHumanReadable() { return stringifyTime(this.parsedTimeRemaining); diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 4469118f065..596ac1c4c1e 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -253,6 +253,7 @@ module ApplicationSettingsHelper :throttle_unauthenticated_enabled, :throttle_unauthenticated_period_in_seconds, :throttle_unauthenticated_requests_per_period, + :time_tracking_display_hours_only, :two_factor_grace_period, :unique_ips_limit_enabled, :unique_ips_limit_per_user, diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 904d650ef96..7b5bf5ad5cf 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -82,6 +82,7 @@ module ApplicationSettingImplementation throttle_unauthenticated_enabled: false, throttle_unauthenticated_period_in_seconds: 3600, throttle_unauthenticated_requests_per_period: 3600, + time_tracking_display_hours_only: false, two_factor_grace_period: 48, unique_ips_limit_enabled: false, unique_ips_limit_per_user: 10, diff --git a/app/views/admin/application_settings/_localization.html.haml b/app/views/admin/application_settings/_localization.html.haml index bb4d1fa1241..8f09d42a053 100644 --- a/app/views/admin/application_settings/_localization.html.haml +++ b/app/views/admin/application_settings/_localization.html.haml @@ -8,4 +8,10 @@ .form-text.text-muted = _('Default first day of the week in calendars and date pickers.') + .form-group + .form-check + = f.check_box :time_tracking_display_hours_only, class: 'form-check-input' + = f.label :time_tracking_display_hours_only, class: 'form-check-label' do + _('Limit time tracking display to hours.') + = f.submit _('Save changes'), class: "btn btn-success" diff --git a/changelogs/unreleased/30355-use-hours-only-for-time-tracking.yml b/changelogs/unreleased/30355-use-hours-only-for-time-tracking.yml new file mode 100644 index 00000000000..afc548472aa --- /dev/null +++ b/changelogs/unreleased/30355-use-hours-only-for-time-tracking.yml @@ -0,0 +1,5 @@ +--- +title: Add option to show time tracking values in hours only +merge_request: 29469 +author: Jon Kolb +type: added diff --git a/db/migrate/20190611090827_add_time_tracking_display_hours_only_to_application_settings.rb b/db/migrate/20190611090827_add_time_tracking_display_hours_only_to_application_settings.rb new file mode 100644 index 00000000000..f4daf0d1863 --- /dev/null +++ b/db/migrate/20190611090827_add_time_tracking_display_hours_only_to_application_settings.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddTimeTrackingDisplayHoursOnlyToApplicationSettings < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default :application_settings, :time_tracking_display_hours_only, :boolean, default: false, allow_null: false + end + + def down + remove_column :application_settings, :time_tracking_display_hours_only + end +end diff --git a/db/schema.rb b/db/schema.rb index b81558178b9..d39833f9755 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -229,6 +229,7 @@ ActiveRecord::Schema.define(version: 20190620112608) do t.integer "custom_project_templates_group_id" t.boolean "elasticsearch_limit_indexing", default: false, null: false t.string "geo_node_allowed_ips", default: "0.0.0.0/0, ::/0" + t.boolean "time_tracking_display_hours_only", default: false, null: false t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id", using: :btree t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id", using: :btree t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree diff --git a/doc/api/settings.md b/doc/api/settings.md index c2a1f7feefd..7363b43393f 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -231,6 +231,7 @@ are listed in the descriptions of the relevant settings. | `throttle_unauthenticated_enabled` | boolean | no | (**If enabled, requires:** `throttle_unauthenticated_period_in_seconds` and `throttle_unauthenticated_requests_per_period`) Enable unauthenticated request rate limit. Helps reduce request volume (e.g. from crawlers or abusive bots). | | `throttle_unauthenticated_period_in_seconds` | integer | required by: `throttle_unauthenticated_enabled` | Rate limit period in seconds. | | `throttle_unauthenticated_requests_per_period` | integer | required by: `throttle_unauthenticated_enabled` | Max requests per period per IP. | +| `time_tracking_display_hours_only` | boolean | no | Limit time tracking values to hours only. Default is `false`. | | `two_factor_grace_period` | integer | required by: `require_two_factor_authentication` | Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication. | | `unique_ips_limit_enabled` | boolean | no | (**If enabled, requires:** `unique_ips_limit_per_user` and `unique_ips_limit_time_window`) Limit sign in from multiple ips. | | `unique_ips_limit_per_user` | integer | required by: `unique_ips_limit_enabled` | Maximum number of ips per user. | diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index 582c3065189..ca2e4e5f001 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -32,6 +32,8 @@ module Gitlab gon.first_day_of_week = current_user&.first_day_of_week || Gitlab::CurrentSettings.first_day_of_week gon.ee = Gitlab.ee? + gon.time_tracking_display_hours_only = Gitlab::CurrentSettings.time_tracking_display_hours_only + if current_user gon.current_user_id = current_user.id gon.current_username = current_user.username diff --git a/lib/gitlab/time_tracking_formatter.rb b/lib/gitlab/time_tracking_formatter.rb index cc206010e74..a63fe51eb97 100644 --- a/lib/gitlab/time_tracking_formatter.rb +++ b/lib/gitlab/time_tracking_formatter.rb @@ -16,7 +16,8 @@ module Gitlab def output(seconds) with_custom_config do - ChronicDuration.output(seconds, format: :short, limit_to_hours: false, weeks: true) rescue nil + limit_to_hours = Gitlab::CurrentSettings.time_tracking_display_hours_only + ChronicDuration.output(seconds, format: :short, limit_to_hours: limit_to_hours, weeks: true) rescue nil end end -- cgit v1.2.1 From 275a17589c2d468d8671a9f754a50b212273d509 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Tue, 11 Jun 2019 18:40:01 +0800 Subject: Rename to time_tracking_limit_to_hours Changes migration and all other places the attribute is used --- .../javascripts/boards/components/board_sidebar.js | 1 + .../boards/components/issue_time_estimate.vue | 15 ++--- app/assets/javascripts/boards/index.js | 1 + .../javascripts/boards/stores/boards_store.js | 7 +++ .../javascripts/lib/utils/datetime_utility.js | 10 +++- .../components/time_tracking/comparison_pane.vue | 7 ++- .../time_tracking/sidebar_time_tracking.vue | 1 + .../components/time_tracking/time_tracker.vue | 5 ++ .../javascripts/sidebar/mount_milestone_sidebar.js | 4 +- .../javascripts/sidebar/stores/sidebar_store.js | 3 +- app/helpers/application_settings_helper.rb | 2 +- app/helpers/boards_helper.rb | 3 +- app/helpers/issuables_helper.rb | 3 +- app/models/application_setting_implementation.rb | 2 +- .../application_settings/_localization.html.haml | 7 ++- .../components/sidebar/_time_tracker.html.haml | 1 + app/views/shared/milestones/_sidebar.html.haml | 6 +- .../30355-use-hours-only-for-time-tracking.yml | 2 +- ...g_display_hours_only_to_application_settings.rb | 21 ------- ...cking_limit_to_hours_to_application_settings.rb | 21 +++++++ db/schema.rb | 2 +- doc/api/settings.md | 2 +- doc/workflow/time_tracking.md | 10 +++- lib/gitlab/gon_helper.rb | 2 - lib/gitlab/time_tracking_formatter.rb | 11 +++- locale/gitlab.pot | 3 + spec/features/boards/sidebar_spec.rb | 22 +++++-- spec/frontend/lib/utils/datetime_utility_spec.js | 6 ++ spec/javascripts/boards/boards_store_spec.js | 10 ++++ .../boards/components/issue_time_estimate_spec.js | 70 +++++++++++++++------- .../components/time_tracking/time_tracker_spec.js | 25 ++++++++ spec/lib/gitlab/time_tracking_formatter_spec.rb | 43 +++++++++++++ spec/services/system_note_service_spec.rb | 24 ++++++++ 33 files changed, 277 insertions(+), 75 deletions(-) delete mode 100644 db/migrate/20190611090827_add_time_tracking_display_hours_only_to_application_settings.rb create mode 100644 db/migrate/20190611090827_add_time_tracking_limit_to_hours_to_application_settings.rb create mode 100644 spec/lib/gitlab/time_tracking_formatter_spec.rb diff --git a/app/assets/javascripts/boards/components/board_sidebar.js b/app/assets/javascripts/boards/components/board_sidebar.js index c587b276fa3..2ace0060c42 100644 --- a/app/assets/javascripts/boards/components/board_sidebar.js +++ b/app/assets/javascripts/boards/components/board_sidebar.js @@ -38,6 +38,7 @@ export default Vue.extend({ issue: {}, list: {}, loadingAssignees: false, + timeTrackingLimitToHours: boardsStore.timeTracking.limitToHours, }; }, computed: { diff --git a/app/assets/javascripts/boards/components/issue_time_estimate.vue b/app/assets/javascripts/boards/components/issue_time_estimate.vue index 2545980163f..3385aad5b11 100644 --- a/app/assets/javascripts/boards/components/issue_time_estimate.vue +++ b/app/assets/javascripts/boards/components/issue_time_estimate.vue @@ -2,6 +2,7 @@ import { GlTooltip } from '@gitlab/ui'; import Icon from '~/vue_shared/components/icon.vue'; import { parseSeconds, stringifyTime } from '~/lib/utils/datetime_utility'; +import boardsStore from '../stores/boards_store'; export default { components: { @@ -14,17 +15,17 @@ export default { required: true, }, }, + data() { + return { + limitToHours: boardsStore.timeTracking.limitToHours, + }; + }, computed: { title() { - return stringifyTime( - parseSeconds(this.estimate, { limitToHours: gon.time_tracking_display_hours_only }), - true - ); + return stringifyTime(parseSeconds(this.estimate, { limitToHours: this.limitToHours }), true); }, timeEstimate() { - return stringifyTime( - parseSeconds(this.estimate, { limitToHours: gon.time_tracking_display_hours_only }) - ); + return stringifyTime(parseSeconds(this.estimate, { limitToHours: this.limitToHours })); }, }, }; diff --git a/app/assets/javascripts/boards/index.js b/app/assets/javascripts/boards/index.js index f2f37d22b97..a020765f335 100644 --- a/app/assets/javascripts/boards/index.js +++ b/app/assets/javascripts/boards/index.js @@ -49,6 +49,7 @@ export default () => { } boardsStore.create(); + boardsStore.setTimeTrackingLimitToHours($boardApp.dataset.timeTrackingLimitToHours); issueBoardsApp = new Vue({ el: $boardApp, diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js index 4b3b44574a8..4ba4cde6bae 100644 --- a/app/assets/javascripts/boards/stores/boards_store.js +++ b/app/assets/javascripts/boards/stores/boards_store.js @@ -12,6 +12,9 @@ import eventHub from '../eventhub'; const boardsStore = { disabled: false, + timeTracking: { + limitToHours: false, + }, scopedLabels: { helpLink: '', enabled: false, @@ -222,6 +225,10 @@ const boardsStore = { setIssueDetail(issueDetail) { this.detail.issue = issueDetail; }, + + setTimeTrackingLimitToHours(limitToHours) { + this.timeTracking.limitToHours = parseBoolean(limitToHours); + }, }; BoardsStoreEE.initEESpecific(boardsStore); diff --git a/app/assets/javascripts/lib/utils/datetime_utility.js b/app/assets/javascripts/lib/utils/datetime_utility.js index f8398f2f780..062d21ed247 100644 --- a/app/assets/javascripts/lib/utils/datetime_utility.js +++ b/app/assets/javascripts/lib/utils/datetime_utility.js @@ -485,6 +485,7 @@ export const parseSeconds = ( ) => { const DAYS_PER_WEEK = daysPerWeek; const HOURS_PER_DAY = hoursPerDay; + const SECONDS_PER_MINUTE = 60; const MINUTES_PER_HOUR = 60; const MINUTES_PER_WEEK = DAYS_PER_WEEK * HOURS_PER_DAY * MINUTES_PER_HOUR; const MINUTES_PER_DAY = HOURS_PER_DAY * MINUTES_PER_HOUR; @@ -496,10 +497,15 @@ export const parseSeconds = ( minutes: 1, }; - let unorderedMinutes = Math.abs(seconds / MINUTES_PER_HOUR); + if (limitToHours) { + timePeriodConstraints.weeks = 0; + timePeriodConstraints.days = 0; + } + + let unorderedMinutes = Math.abs(seconds / SECONDS_PER_MINUTE); return _.mapObject(timePeriodConstraints, minutesPerPeriod => { - if (limitToHours && minutesPerPeriod > MINUTES_PER_HOUR) { + if (minutesPerPeriod === 0) { return 0; } diff --git a/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue b/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue index cd3e7ce33f7..13955529cab 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue @@ -28,11 +28,16 @@ export default { type: String, required: true, }, + limitToHours: { + type: Boolean, + required: false, + default: false, + }, }, computed: { parsedTimeRemaining() { const diffSeconds = this.timeEstimate - this.timeSpent; - return parseSeconds(diffSeconds, { limitToHours: gon.time_tracking_display_hours_only }); + return parseSeconds(diffSeconds, { limitToHours: this.limitToHours }); }, timeRemainingHumanReadable() { return stringifyTime(this.parsedTimeRemaining); diff --git a/app/assets/javascripts/sidebar/components/time_tracking/sidebar_time_tracking.vue b/app/assets/javascripts/sidebar/components/time_tracking/sidebar_time_tracking.vue index 8e8b9f19b6e..018b30d2a67 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/sidebar_time_tracking.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/sidebar_time_tracking.vue @@ -53,6 +53,7 @@ export default { :time-spent="store.totalTimeSpent" :human-time-estimate="store.humanTimeEstimate" :human-time-spent="store.humanTotalTimeSpent" + :limit-to-hours="store.timeTrackingLimitToHours" :root-path="store.rootPath" /> diff --git a/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue b/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue index d84d5344935..682ca600b6a 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue @@ -37,6 +37,10 @@ export default { required: false, default: '', }, + limitToHours: { + type: Boolean, + default: false, + }, rootPath: { type: String, required: true, @@ -129,6 +133,7 @@ export default { :time-spent="timeSpent" :time-spent-human-readable="humanTimeSpent" :time-estimate-human-readable="humanTimeEstimate" + :limit-to-hours="limitToHours" /> diff --git a/app/assets/javascripts/sidebar/mount_milestone_sidebar.js b/app/assets/javascripts/sidebar/mount_milestone_sidebar.js index 1ebdbec7bc9..d934463382f 100644 --- a/app/assets/javascripts/sidebar/mount_milestone_sidebar.js +++ b/app/assets/javascripts/sidebar/mount_milestone_sidebar.js @@ -1,5 +1,6 @@ import Vue from 'vue'; import timeTracker from './components/time_tracking/time_tracker.vue'; +import { parseBoolean } from '~/lib/utils/common_utils'; export default class SidebarMilestone { constructor() { @@ -7,7 +8,7 @@ export default class SidebarMilestone { if (!el) return; - const { timeEstimate, timeSpent, humanTimeEstimate, humanTimeSpent } = el.dataset; + const { timeEstimate, timeSpent, humanTimeEstimate, humanTimeSpent, limitToHours } = el.dataset; // eslint-disable-next-line no-new new Vue({ @@ -22,6 +23,7 @@ export default class SidebarMilestone { timeSpent: parseInt(timeSpent, 10), humanTimeEstimate, humanTimeSpent, + limitToHours: parseBoolean(limitToHours), rootPath: '/', }, }), diff --git a/app/assets/javascripts/sidebar/stores/sidebar_store.js b/app/assets/javascripts/sidebar/stores/sidebar_store.js index 7b8b4c5d856..63c4a2a3f84 100644 --- a/app/assets/javascripts/sidebar/stores/sidebar_store.js +++ b/app/assets/javascripts/sidebar/stores/sidebar_store.js @@ -8,7 +8,7 @@ export default class SidebarStore { } initSingleton(options) { - const { currentUser, rootPath, editable } = options; + const { currentUser, rootPath, editable, timeTrackingLimitToHours } = options; this.currentUser = currentUser; this.rootPath = rootPath; this.editable = editable; @@ -16,6 +16,7 @@ export default class SidebarStore { this.totalTimeSpent = 0; this.humanTimeEstimate = ''; this.humanTimeSpent = ''; + this.timeTrackingLimitToHours = timeTrackingLimitToHours; this.assignees = []; this.isFetching = { assignees: true, diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 596ac1c4c1e..d837c42fd68 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -253,7 +253,7 @@ module ApplicationSettingsHelper :throttle_unauthenticated_enabled, :throttle_unauthenticated_period_in_seconds, :throttle_unauthenticated_requests_per_period, - :time_tracking_display_hours_only, + :time_tracking_limit_to_hours, :two_factor_grace_period, :unique_ips_limit_enabled, :unique_ips_limit_per_user, diff --git a/app/helpers/boards_helper.rb b/app/helpers/boards_helper.rb index 1640f4fc93f..c5130b430b9 100644 --- a/app/helpers/boards_helper.rb +++ b/app/helpers/boards_helper.rb @@ -14,7 +14,8 @@ module BoardsHelper issue_link_base: build_issue_link_base, root_path: root_path, bulk_update_path: @bulk_issues_path, - default_avatar: image_path(default_avatar) + default_avatar: image_path(default_avatar), + time_tracking_limit_to_hours: Gitlab::CurrentSettings.time_tracking_limit_to_hours.to_s } end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 150f24a5d5b..045de105b77 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -430,7 +430,8 @@ module IssuablesHelper editable: issuable.dig(:current_user, :can_edit), currentUser: issuable[:current_user], rootPath: root_path, - fullPath: issuable[:project_full_path] + fullPath: issuable[:project_full_path], + timeTrackingLimitToHours: Gitlab::CurrentSettings.time_tracking_limit_to_hours } end diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index 7b5bf5ad5cf..cf328bcd994 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -82,7 +82,7 @@ module ApplicationSettingImplementation throttle_unauthenticated_enabled: false, throttle_unauthenticated_period_in_seconds: 3600, throttle_unauthenticated_requests_per_period: 3600, - time_tracking_display_hours_only: false, + time_tracking_limit_to_hours: false, two_factor_grace_period: 48, unique_ips_limit_enabled: false, unique_ips_limit_per_user: 10, diff --git a/app/views/admin/application_settings/_localization.html.haml b/app/views/admin/application_settings/_localization.html.haml index 8f09d42a053..e01c123d1db 100644 --- a/app/views/admin/application_settings/_localization.html.haml +++ b/app/views/admin/application_settings/_localization.html.haml @@ -9,9 +9,10 @@ = _('Default first day of the week in calendars and date pickers.') .form-group + = f.label :time_tracking, _('Time tracking'), class: 'label-bold' .form-check - = f.check_box :time_tracking_display_hours_only, class: 'form-check-input' - = f.label :time_tracking_display_hours_only, class: 'form-check-label' do - _('Limit time tracking display to hours.') + = f.check_box :time_tracking_limit_to_hours, class: 'form-check-input' + = f.label :time_tracking_limit_to_hours, class: 'form-check-label' do + = _('Limit display of time tracking units to hours.') = f.submit _('Save changes'), class: "btn btn-success" diff --git a/app/views/shared/boards/components/sidebar/_time_tracker.html.haml b/app/views/shared/boards/components/sidebar/_time_tracker.html.haml index b76d44c5907..43081499920 100644 --- a/app/views/shared/boards/components/sidebar/_time_tracker.html.haml +++ b/app/views/shared/boards/components/sidebar/_time_tracker.html.haml @@ -3,4 +3,5 @@ ":time-spent" => "issue.timeSpent || 0", ":human-time-estimate" => "issue.humanTimeEstimate", ":human-time-spent" => "issue.humanTimeSpent", + ":limit-to-hours" => "timeTrackingLimitToHours", "root-path" => "#{root_url}" } diff --git a/app/views/shared/milestones/_sidebar.html.haml b/app/views/shared/milestones/_sidebar.html.haml index b24075c7849..ced6af50501 100644 --- a/app/views/shared/milestones/_sidebar.html.haml +++ b/app/views/shared/milestones/_sidebar.html.haml @@ -93,7 +93,11 @@ = milestone.issues_visible_to_user(current_user).closed.count .block - #issuable-time-tracker{ data: { time_estimate: @milestone.total_issue_time_estimate, time_spent: @milestone.total_issue_time_spent, human_time_estimate: @milestone.human_total_issue_time_estimate, human_time_spent: @milestone.human_total_issue_time_spent } } + #issuable-time-tracker{ data: { time_estimate: @milestone.total_issue_time_estimate, + time_spent: @milestone.total_issue_time_spent, + human_time_estimate: @milestone.human_total_issue_time_estimate, + human_time_spent: @milestone.human_total_issue_time_spent, + limit_to_hours: Gitlab::CurrentSettings.time_tracking_limit_to_hours.to_s } } // Fallback while content is loading .title.hide-collapsed = _('Time tracking') diff --git a/changelogs/unreleased/30355-use-hours-only-for-time-tracking.yml b/changelogs/unreleased/30355-use-hours-only-for-time-tracking.yml index afc548472aa..b0252f9e81b 100644 --- a/changelogs/unreleased/30355-use-hours-only-for-time-tracking.yml +++ b/changelogs/unreleased/30355-use-hours-only-for-time-tracking.yml @@ -1,5 +1,5 @@ --- -title: Add option to show time tracking values in hours only +title: Add option to limit time tracking units to hours merge_request: 29469 author: Jon Kolb type: added diff --git a/db/migrate/20190611090827_add_time_tracking_display_hours_only_to_application_settings.rb b/db/migrate/20190611090827_add_time_tracking_display_hours_only_to_application_settings.rb deleted file mode 100644 index f4daf0d1863..00000000000 --- a/db/migrate/20190611090827_add_time_tracking_display_hours_only_to_application_settings.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddTimeTrackingDisplayHoursOnlyToApplicationSettings < ActiveRecord::Migration[5.1] - include Gitlab::Database::MigrationHelpers - - # Set this constant to true if this migration requires downtime. - DOWNTIME = false - - disable_ddl_transaction! - - def up - add_column_with_default :application_settings, :time_tracking_display_hours_only, :boolean, default: false, allow_null: false - end - - def down - remove_column :application_settings, :time_tracking_display_hours_only - end -end diff --git a/db/migrate/20190611090827_add_time_tracking_limit_to_hours_to_application_settings.rb b/db/migrate/20190611090827_add_time_tracking_limit_to_hours_to_application_settings.rb new file mode 100644 index 00000000000..a5f8925c1db --- /dev/null +++ b/db/migrate/20190611090827_add_time_tracking_limit_to_hours_to_application_settings.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddTimeTrackingLimitToHoursToApplicationSettings < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default :application_settings, :time_tracking_limit_to_hours, :boolean, default: false, allow_null: false + end + + def down + remove_column :application_settings, :time_tracking_limit_to_hours + end +end diff --git a/db/schema.rb b/db/schema.rb index d39833f9755..44213fddd86 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -229,7 +229,7 @@ ActiveRecord::Schema.define(version: 20190620112608) do t.integer "custom_project_templates_group_id" t.boolean "elasticsearch_limit_indexing", default: false, null: false t.string "geo_node_allowed_ips", default: "0.0.0.0/0, ::/0" - t.boolean "time_tracking_display_hours_only", default: false, null: false + t.boolean "time_tracking_limit_to_hours", default: false, null: false t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id", using: :btree t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id", using: :btree t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree diff --git a/doc/api/settings.md b/doc/api/settings.md index 7363b43393f..eb3f39e6670 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -231,7 +231,7 @@ are listed in the descriptions of the relevant settings. | `throttle_unauthenticated_enabled` | boolean | no | (**If enabled, requires:** `throttle_unauthenticated_period_in_seconds` and `throttle_unauthenticated_requests_per_period`) Enable unauthenticated request rate limit. Helps reduce request volume (e.g. from crawlers or abusive bots). | | `throttle_unauthenticated_period_in_seconds` | integer | required by: `throttle_unauthenticated_enabled` | Rate limit period in seconds. | | `throttle_unauthenticated_requests_per_period` | integer | required by: `throttle_unauthenticated_enabled` | Max requests per period per IP. | -| `time_tracking_display_hours_only` | boolean | no | Limit time tracking values to hours only. Default is `false`. | +| `time_tracking_limit_to_hours` | boolean | no | Limit display of time tracking units to hours. Default is `false`. | | `two_factor_grace_period` | integer | required by: `require_two_factor_authentication` | Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication. | | `unique_ips_limit_enabled` | boolean | no | (**If enabled, requires:** `unique_ips_limit_per_user` and `unique_ips_limit_time_window`) Limit sign in from multiple ips. | | `unique_ips_limit_per_user` | integer | required by: `unique_ips_limit_enabled` | Maximum number of ips per user. | diff --git a/doc/workflow/time_tracking.md b/doc/workflow/time_tracking.md index c03dffa967d..4286a3625a2 100644 --- a/doc/workflow/time_tracking.md +++ b/doc/workflow/time_tracking.md @@ -73,7 +73,15 @@ The following time units are available: Default conversion rates are 1mo = 4w, 1w = 5d and 1d = 8h. -Other interesting links: +### Limit displayed units to hours + +> Introduced in GitLab 12.0. + +The display of time units can be limited to hours through the option in **Admin Area > Settings > Preferences** under 'Localization'. + +With this option enabled, `75h` is displayed instead of `1w 4d 3h`. + +## Other interesting links - [Time Tracking landing page on about.gitlab.com](https://about.gitlab.com/solutions/time-tracking/) diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index ca2e4e5f001..582c3065189 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -32,8 +32,6 @@ module Gitlab gon.first_day_of_week = current_user&.first_day_of_week || Gitlab::CurrentSettings.first_day_of_week gon.ee = Gitlab.ee? - gon.time_tracking_display_hours_only = Gitlab::CurrentSettings.time_tracking_display_hours_only - if current_user gon.current_user_id = current_user.id gon.current_username = current_user.username diff --git a/lib/gitlab/time_tracking_formatter.rb b/lib/gitlab/time_tracking_formatter.rb index a63fe51eb97..302da91328a 100644 --- a/lib/gitlab/time_tracking_formatter.rb +++ b/lib/gitlab/time_tracking_formatter.rb @@ -6,7 +6,7 @@ module Gitlab def parse(string) with_custom_config do - string.sub!(/\A-/, '') + string = string.sub(/\A-/, '') seconds = ChronicDuration.parse(string, default_unit: 'hours') rescue nil seconds *= -1 if seconds && Regexp.last_match @@ -16,11 +16,12 @@ module Gitlab def output(seconds) with_custom_config do - limit_to_hours = Gitlab::CurrentSettings.time_tracking_display_hours_only - ChronicDuration.output(seconds, format: :short, limit_to_hours: limit_to_hours, weeks: true) rescue nil + ChronicDuration.output(seconds, format: :short, limit_to_hours: limit_to_hours_setting, weeks: true) rescue nil end end + private + def with_custom_config # We may want to configure it through project settings in a future version. ChronicDuration.hours_per_day = 8 @@ -33,5 +34,9 @@ module Gitlab result end + + def limit_to_hours_setting + Gitlab::CurrentSettings.time_tracking_limit_to_hours + end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 97613a6a920..68727320998 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5839,6 +5839,9 @@ msgstr "" msgid "Let's Encrypt is a free, automated, and open certificate authority (CA) that gives digital certificates in order to enable HTTPS (SSL/TLS) for websites. Learn more about Let's Encrypt configuration by following the %{docs_link_start}documentation on GitLab Pages%{docs_link_end}." msgstr "" +msgid "Limit display of time tracking units to hours." +msgstr "" + msgid "Limited to showing %d event at most" msgid_plural "Limited to showing %d events at most" msgstr[0] "" diff --git a/spec/features/boards/sidebar_spec.rb b/spec/features/boards/sidebar_spec.rb index b1798c11361..6c9ae343e01 100644 --- a/spec/features/boards/sidebar_spec.rb +++ b/spec/features/boards/sidebar_spec.rb @@ -16,7 +16,9 @@ describe 'Issue Boards', :js do let!(:issue2) { create(:labeled_issue, project: project, labels: [development, stretch], relative_position: 1) } let(:board) { create(:board, project: project) } let!(:list) { create(:list, board: board, label: development, position: 0) } - let(:card) { find('.board:nth-child(2)').first('.board-card') } + let(:card) { find('.board:nth-child(2)').first('.board-card') } + + let(:application_settings) { {} } around do |example| Timecop.freeze { example.run } @@ -27,6 +29,8 @@ describe 'Issue Boards', :js do sign_in(user) + stub_application_setting(application_settings) + visit project_board_path(project, board) wait_for_requests end @@ -223,16 +227,24 @@ describe 'Issue Boards', :js do end context 'time tracking' do + let(:compare_meter_tooltip) { find('.time-tracking .time-tracking-content .compare-meter')['data-original-title'] } + before do issue2.timelogs.create(time_spent: 14400, user: user) - issue2.update!(time_estimate: 28800) + issue2.update!(time_estimate: 128800) + + click_card(card) end it 'shows time tracking progress bar' do - click_card(card) + expect(compare_meter_tooltip).to eq('Time remaining: 3d 7h 46m') + end + + context 'when time_tracking_limit_to_hours is true' do + let(:application_settings) { { time_tracking_limit_to_hours: true } } - page.within('.time-tracking') do - expect(find('.time-tracking-content .compare-meter')['data-original-title']).to eq('Time remaining: 4h') + it 'shows time tracking progress bar' do + expect(compare_meter_tooltip).to eq('Time remaining: 31h 46m') end end end diff --git a/spec/frontend/lib/utils/datetime_utility_spec.js b/spec/frontend/lib/utils/datetime_utility_spec.js index 9f49e68cfe8..751fb5e1b94 100644 --- a/spec/frontend/lib/utils/datetime_utility_spec.js +++ b/spec/frontend/lib/utils/datetime_utility_spec.js @@ -334,6 +334,12 @@ describe('prettyTime methods', () => { assertTimeUnits(aboveOneDay, 33, 2, 2, 0); assertTimeUnits(aboveOneWeek, 26, 0, 1, 9); }); + + it('should correctly parse values when limitedToHours is true', () => { + const twoDays = datetimeUtility.parseSeconds(173000, { limitToHours: true }); + + assertTimeUnits(twoDays, 3, 48, 0, 0); + }); }); describe('stringifyTime', () => { diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js index e81115e10c9..5266b1bdbfc 100644 --- a/spec/javascripts/boards/boards_store_spec.js +++ b/spec/javascripts/boards/boards_store_spec.js @@ -355,4 +355,14 @@ describe('Store', () => { expect(boardsStore.moving.list).toEqual(dummyList); }); }); + + describe('setTimeTrackingLimitToHours', () => { + it('sets the timeTracking.LimitToHours option', () => { + boardsStore.timeTracking.limitToHours = false; + + boardsStore.setTimeTrackingLimitToHours('true'); + + expect(boardsStore.timeTracking.limitToHours).toEqual(true); + }); + }); }); diff --git a/spec/javascripts/boards/components/issue_time_estimate_spec.js b/spec/javascripts/boards/components/issue_time_estimate_spec.js index ba65d3287da..de48e3f6091 100644 --- a/spec/javascripts/boards/components/issue_time_estimate_spec.js +++ b/spec/javascripts/boards/components/issue_time_estimate_spec.js @@ -1,40 +1,70 @@ import Vue from 'vue'; import IssueTimeEstimate from '~/boards/components/issue_time_estimate.vue'; +import boardsStore from '~/boards/stores/boards_store'; import mountComponent from '../../helpers/vue_mount_component_helper'; -describe('Issue Tine Estimate component', () => { +describe('Issue Time Estimate component', () => { let vm; beforeEach(() => { - const Component = Vue.extend(IssueTimeEstimate); - vm = mountComponent(Component, { - estimate: 374460, - }); + boardsStore.create(); }); afterEach(() => { vm.$destroy(); }); - it('renders the correct time estimate', () => { - expect(vm.$el.querySelector('time').textContent.trim()).toEqual('2w 3d 1m'); - }); + describe('when limitToHours is false', () => { + beforeEach(() => { + boardsStore.timeTracking.limitToHours = false; + + const Component = Vue.extend(IssueTimeEstimate); + vm = mountComponent(Component, { + estimate: 374460, + }); + }); + + it('renders the correct time estimate', () => { + expect(vm.$el.querySelector('time').textContent.trim()).toEqual('2w 3d 1m'); + }); + + it('renders expanded time estimate in tooltip', () => { + expect(vm.$el.querySelector('.js-issue-time-estimate').textContent).toContain( + '2 weeks 3 days 1 minute', + ); + }); + + it('prevents tooltip xss', done => { + const alertSpy = spyOn(window, 'alert'); + vm.estimate = 'Foo '; - it('renders expanded time estimate in tooltip', () => { - expect(vm.$el.querySelector('.js-issue-time-estimate').textContent).toContain( - '2 weeks 3 days 1 minute', - ); + vm.$nextTick(() => { + expect(alertSpy).not.toHaveBeenCalled(); + expect(vm.$el.querySelector('time').textContent.trim()).toEqual('0m'); + expect(vm.$el.querySelector('.js-issue-time-estimate').textContent).toContain('0m'); + done(); + }); + }); }); - it('prevents tooltip xss', done => { - const alertSpy = spyOn(window, 'alert'); - vm.estimate = 'Foo '; + describe('when limitToHours is true', () => { + beforeEach(() => { + boardsStore.timeTracking.limitToHours = true; + + const Component = Vue.extend(IssueTimeEstimate); + vm = mountComponent(Component, { + estimate: 374460, + }); + }); + + it('renders the correct time estimate', () => { + expect(vm.$el.querySelector('time').textContent.trim()).toEqual('104h 1m'); + }); - vm.$nextTick(() => { - expect(alertSpy).not.toHaveBeenCalled(); - expect(vm.$el.querySelector('time').textContent.trim()).toEqual('0m'); - expect(vm.$el.querySelector('.js-issue-time-estimate').textContent).toContain('0m'); - done(); + it('renders expanded time estimate in tooltip', () => { + expect(vm.$el.querySelector('.js-issue-time-estimate').textContent).toContain( + '104 hours 1 minute', + ); }); }); }); diff --git a/spec/javascripts/sidebar/components/time_tracking/time_tracker_spec.js b/spec/javascripts/sidebar/components/time_tracking/time_tracker_spec.js index 4c3dd713589..2e1863cff86 100644 --- a/spec/javascripts/sidebar/components/time_tracking/time_tracker_spec.js +++ b/spec/javascripts/sidebar/components/time_tracking/time_tracker_spec.js @@ -13,6 +13,7 @@ describe('Issuable Time Tracker', () => { timeSpent, timeEstimateHumanReadable, timeSpentHumanReadable, + limitToHours, }) => { setFixtures(`
@@ -25,6 +26,7 @@ describe('Issuable Time Tracker', () => { timeSpent, humanTimeEstimate: timeEstimateHumanReadable, humanTimeSpent: timeSpentHumanReadable, + limitToHours: Boolean(limitToHours), rootPath: '/', }; @@ -128,6 +130,29 @@ describe('Issuable Time Tracker', () => { }); }); + describe('Comparison pane when limitToHours is true', () => { + beforeEach(() => { + initTimeTrackingComponent({ + timeEstimate: 100000, // 1d 3h + timeSpent: 5000, // 1h 23m + timeEstimateHumanReadable: '', + timeSpentHumanReadable: '', + limitToHours: true, + }); + }); + + it('should show the correct tooltip text', done => { + Vue.nextTick(() => { + expect(vm.showComparisonState).toBe(true); + const $title = vm.$el.querySelector('.time-tracking-content .compare-meter').dataset + .originalTitle; + + expect($title).toBe('Time remaining: 26h 23m'); + done(); + }); + }); + }); + describe('Estimate only pane', () => { beforeEach(() => { initTimeTrackingComponent({ diff --git a/spec/lib/gitlab/time_tracking_formatter_spec.rb b/spec/lib/gitlab/time_tracking_formatter_spec.rb new file mode 100644 index 00000000000..a85d418777f --- /dev/null +++ b/spec/lib/gitlab/time_tracking_formatter_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::TimeTrackingFormatter do + describe '#parse' do + subject { described_class.parse(duration_string) } + + context 'positive durations' do + let(:duration_string) { '3h 20m' } + + it { expect(subject).to eq(12_000) } + end + + context 'negative durations' do + let(:duration_string) { '-3h 20m' } + + it { expect(subject).to eq(-12_000) } + end + end + + describe '#output' do + let(:num_seconds) { 178_800 } + + subject { described_class.output(num_seconds) } + + context 'time_tracking_limit_to_hours setting is true' do + before do + stub_application_setting(time_tracking_limit_to_hours: true) + end + + it { expect(subject).to eq('49h 40m') } + end + + context 'time_tracking_limit_to_hours setting is false' do + before do + stub_application_setting(time_tracking_limit_to_hours: false) + end + + it { expect(subject).to eq('1w 1d 1h 40m') } + end + end +end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 2420817e1f7..30a867fa7ba 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -946,6 +946,18 @@ describe SystemNoteService do expect(subject.note).to eq "changed time estimate to 1w 4d 5h" end + + context 'when time_tracking_limit_to_hours setting is true' do + before do + stub_application_setting(time_tracking_limit_to_hours: true) + end + + it 'sets the note text' do + noteable.update_attribute(:time_estimate, 277200) + + expect(subject.note).to eq "changed time estimate to 77h" + end + end end context 'without a time estimate' do @@ -1022,6 +1034,18 @@ describe SystemNoteService do end end + context 'when time_tracking_limit_to_hours setting is true' do + before do + stub_application_setting(time_tracking_limit_to_hours: true) + end + + it 'sets the note text' do + spend_time!(277200) + + expect(subject.note).to eq "added 77h of time spent" + end + end + def spend_time!(seconds) noteable.spend_time(duration: seconds, user_id: author.id) noteable.save! -- cgit v1.2.1