From 38333620c30a22ee1ae7b4c916620a0d1afe6c30 Mon Sep 17 00:00:00 2001 From: Marco Zille Date: Wed, 19 Oct 2022 22:51:08 +0200 Subject: [PATCH 1/9] Added a UI to create timelogs Changelog: added --- .../sidebar/board_sidebar_time_tracker.vue | 3 +- .../time_tracking/create_timelog_form.vue | 177 +++++++++++++++ .../time_tracking/sidebar_time_tracking.vue | 6 + .../components/time_tracking/time_tracker.vue | 37 +-- .../sidebar/mount_milestone_sidebar.js | 1 + .../javascripts/sidebar/mount_sidebar.js | 11 +- .../queries/create_timelog.mutation.graphql | 17 ++ app/graphql/mutations/timelogs/create.rb | 2 +- app/helpers/issuables_helper.rb | 1 + .../issuable_sidebar_basic_entity.rb | 4 + doc/api/graphql/reference/index.md | 2 +- .../graphql/mutations/timelogs/create_spec.rb | 10 - .../board_sidebar_time_tracker_spec.js | 15 +- .../time_tracking/create_timelog_form_spec.js | 211 ++++++++++++++++++ .../time_tracking/time_tracker_spec.js | 63 ++---- .../graphql/mutations/timelogs/create_spec.rb | 8 - .../timelogs/create_shared_examples.rb | 25 ++- 17 files changed, 509 insertions(+), 84 deletions(-) create mode 100644 app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue create mode 100644 app/assets/javascripts/sidebar/queries/create_timelog.mutation.graphql create mode 100644 spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js diff --git a/app/assets/javascripts/boards/components/sidebar/board_sidebar_time_tracker.vue b/app/assets/javascripts/boards/components/sidebar/board_sidebar_time_tracker.vue index a35b3f14be4254..b70294c9db3755 100644 --- a/app/assets/javascripts/boards/components/sidebar/board_sidebar_time_tracker.vue +++ b/app/assets/javascripts/boards/components/sidebar/board_sidebar_time_tracker.vue @@ -6,7 +6,7 @@ export default { components: { IssuableTimeTracker, }, - inject: ['timeTrackingLimitToHours'], + inject: ['timeTrackingLimitToHours', 'canUpdate'], computed: { ...mapGetters(['activeBoardItem']), initialTimeTracking() { @@ -34,5 +34,6 @@ export default { :limit-to-hours="timeTrackingLimitToHours" :initial-time-tracking="initialTimeTracking" :show-collapsed="false" + :can-add-time-entries="canUpdate" /> diff --git a/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue b/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue new file mode 100644 index 00000000000000..7f9140df43a11c --- /dev/null +++ b/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue @@ -0,0 +1,177 @@ + + + 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 62b054218843db..06adc048942b58 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 @@ -30,6 +30,11 @@ export default { required: false, default: false, }, + canAddTimeEntries: { + type: Boolean, + required: false, + default: true, + }, }, mounted() { this.listenForQuickActions(); @@ -67,6 +72,7 @@ export default { :issuable-id="issuableId" :issuable-iid="issuableIid" :limit-to-hours="limitToHours" + :can-add-time-entries="canAddTimeEntries" /> 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 3d4939490a36e5..017aafe71878cf 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue @@ -9,15 +9,16 @@ import { GlTooltipDirective, } from '@gitlab/ui'; import { IssuableType } from '~/issues/constants'; +import { BV_SHOW_MODAL } from '~/lib/utils/constants'; import { s__, __ } from '~/locale'; import { HOW_TO_TRACK_TIME, timeTrackingQueries } from '../../constants'; import eventHub from '../../event_hub'; import TimeTrackingCollapsedState from './collapsed_state.vue'; import TimeTrackingComparisonPane from './comparison_pane.vue'; -import TimeTrackingHelpState from './help_state.vue'; import TimeTrackingReport from './report.vue'; import TimeTrackingSpentOnlyPane from './spent_only_pane.vue'; +import CreateTimelogForm from './create_timelog_form.vue'; export default { name: 'IssuableTimeTracker', @@ -34,8 +35,8 @@ export default { TimeTrackingCollapsedState, TimeTrackingSpentOnlyPane, TimeTrackingComparisonPane, - TimeTrackingHelpState, TimeTrackingReport, + CreateTimelogForm, }, directives: { GlModal: GlModalDirective, @@ -87,6 +88,11 @@ export default { default: true, required: false, }, + canAddTimeEntries: { + type: Boolean, + required: false, + default: true, + }, }, data() { return { @@ -192,12 +198,12 @@ export default { eventHub.$on('timeTracker:refresh', this.refresh); }, methods: { - toggleHelpState(show) { - this.showHelp = show; - }, refresh() { this.$apollo.queries.issuableTimeTracking.refetch(); }, + openRegisterTimeSpentModal() { + this.$root.$emit(BV_SHOW_MODAL, 'create-timelog-modal'); + }, }, }; @@ -215,24 +221,21 @@ export default { :time-estimate-human-readable="humanTimeEstimate" />
{{ __('Time tracking') }} - +
@@ -272,9 +275,7 @@ export default { - - - +
diff --git a/app/assets/javascripts/sidebar/mount_milestone_sidebar.js b/app/assets/javascripts/sidebar/mount_milestone_sidebar.js index afce59d304f5b6..b908cf0cd9e541 100644 --- a/app/assets/javascripts/sidebar/mount_milestone_sidebar.js +++ b/app/assets/javascripts/sidebar/mount_milestone_sidebar.js @@ -39,6 +39,7 @@ export default class SidebarMilestone { humanTimeEstimate, humanTotalTimeSpent: humanTimeSpent, }, + canAddTimeEntries: false, }, }), }); diff --git a/app/assets/javascripts/sidebar/mount_sidebar.js b/app/assets/javascripts/sidebar/mount_sidebar.js index 5127db6368d1d1..a308dc8d13c36d 100644 --- a/app/assets/javascripts/sidebar/mount_sidebar.js +++ b/app/assets/javascripts/sidebar/mount_sidebar.js @@ -544,7 +544,15 @@ function mountSidebarSubscriptionsWidget() { function mountSidebarTimeTracking() { const el = document.querySelector('.js-sidebar-time-tracking-root'); - const { id, iid, fullPath, issuableType, timeTrackingLimitToHours } = getSidebarOptions(); + + const { + id, + iid, + fullPath, + issuableType, + timeTrackingLimitToHours, + canCreateTimelogs, + } = getSidebarOptions(); if (!el) { return null; @@ -562,6 +570,7 @@ function mountSidebarTimeTracking() { issuableId: id.toString(), issuableIid: iid.toString(), limitToHours: timeTrackingLimitToHours, + canAddTimeEntries: canCreateTimelogs, }, }), }); diff --git a/app/assets/javascripts/sidebar/queries/create_timelog.mutation.graphql b/app/assets/javascripts/sidebar/queries/create_timelog.mutation.graphql new file mode 100644 index 00000000000000..a8692387a46bba --- /dev/null +++ b/app/assets/javascripts/sidebar/queries/create_timelog.mutation.graphql @@ -0,0 +1,17 @@ +#import "~/graphql_shared/fragments/issue_time_tracking.fragment.graphql" +#import "~/graphql_shared/fragments/merge_request_time_tracking.fragment.graphql" + +mutation createTimelog($input: TimelogCreateInput!) { + timelogCreate(input: $input) { + errors + timelog { + id + issue { + ...IssueTimeTrackingFragment + } + mergeRequest { + ...MergeRequestTimeTrackingFragment + } + } + } +} diff --git a/app/graphql/mutations/timelogs/create.rb b/app/graphql/mutations/timelogs/create.rb index bab7508454e371..cf4fad56ec9f73 100644 --- a/app/graphql/mutations/timelogs/create.rb +++ b/app/graphql/mutations/timelogs/create.rb @@ -11,7 +11,7 @@ class Create < Base description: 'Amount of time spent.' argument :spent_at, - Types::DateType, + Types::TimeType, required: true, description: 'When the time was spent.' diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index c43cca3a81d35f..e0bf4e8a759501 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -412,6 +412,7 @@ def issuable_sidebar_options(issuable) id: issuable[:id], severity: issuable[:severity], timeTrackingLimitToHours: Gitlab::CurrentSettings.time_tracking_limit_to_hours, + canCreateTimelogs: issuable.dig(:current_user, :can_create_timelogs), createNoteEmail: issuable[:create_note_email], issuableType: issuable[:type] } diff --git a/app/serializers/issuable_sidebar_basic_entity.rb b/app/serializers/issuable_sidebar_basic_entity.rb index b66aad6cc6522b..9039606a8e59a0 100644 --- a/app/serializers/issuable_sidebar_basic_entity.rb +++ b/app/serializers/issuable_sidebar_basic_entity.rb @@ -38,6 +38,10 @@ class IssuableSidebarBasicEntity < Grape::Entity expose :can_admin_label do |issuable| can?(current_user, :admin_label, issuable.project) end + + expose :can_create_timelogs do |issuable| + can?(current_user, :create_timelog, issuable) + end end expose :issuable_json_path do |issuable| diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 7feaddf3e2655b..a8c1b39ba7c9c6 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -5106,7 +5106,7 @@ Input type: `TimelogCreateInput` | ---- | ---- | ----------- | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | | `issuableId` | [`IssuableID!`](#issuableid) | Global ID of the issuable (Issue, WorkItem or MergeRequest). | -| `spentAt` | [`Date!`](#date) | When the time was spent. | +| `spentAt` | [`Time!`](#time) | When the time was spent. | | `summary` | [`String!`](#string) | Summary of time spent. | | `timeSpent` | [`String!`](#string) | Amount of time spent. | diff --git a/ee/spec/requests/api/graphql/mutations/timelogs/create_spec.rb b/ee/spec/requests/api/graphql/mutations/timelogs/create_spec.rb index af8855a8334b07..1e3491882f63f4 100644 --- a/ee/spec/requests/api/graphql/mutations/timelogs/create_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/timelogs/create_spec.rb @@ -12,16 +12,6 @@ let(:current_user) { nil } let(:users_container) { group } - let(:mutation) do - graphql_mutation(:timelogCreate, { - 'time_spent' => time_spent, - 'spent_at' => '2022-07-08', - 'summary' => 'Test summary', - 'issuable_id' => issuable.to_global_id.to_s - }) - end - - let(:mutation_response) { graphql_mutation_response(:timelog_create) } context 'when issuable is an Epic' do let_it_be(:issuable) { create(:epic, group: group) } diff --git a/spec/frontend/boards/components/sidebar/board_sidebar_time_tracker_spec.js b/spec/frontend/boards/components/sidebar/board_sidebar_time_tracker_spec.js index 5c4356434255cd..e2e4baefad00a4 100644 --- a/spec/frontend/boards/components/sidebar/board_sidebar_time_tracker_spec.js +++ b/spec/frontend/boards/components/sidebar/board_sidebar_time_tracker_spec.js @@ -42,13 +42,20 @@ describe('BoardSidebarTimeTracker', () => { wrapper = null; }); - it.each([[true], [false]])( - 'renders IssuableTimeTracker with correct spent and estimated time (timeTrackingLimitToHours=%s)', - (timeTrackingLimitToHours) => { - createComponent({ provide: { timeTrackingLimitToHours } }); + it.each` + timeTrackingLimitToHours | canUpdate + ${true} | ${false} + ${true} | ${true} + ${false} | ${false} + ${false} | ${true} + `( + 'renders IssuableTimeTracker with correct spent and estimated time (timeTrackingLimitToHours=$timeTrackingLimitToHours, canUpdate=$canUpdate)', + ({ timeTrackingLimitToHours, canUpdate }) => { + createComponent({ provide: { timeTrackingLimitToHours, canUpdate } }); expect(wrapper.findComponent(IssuableTimeTracker).props()).toEqual({ limitToHours: timeTrackingLimitToHours, + canAddTimeEntries: canUpdate, showCollapsed: false, issuableId: '1', issuableIid: '1', diff --git a/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js b/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js new file mode 100644 index 00000000000000..eb2ea00c0ef003 --- /dev/null +++ b/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js @@ -0,0 +1,211 @@ +import Vue, { nextTick } from 'vue'; +import VueApollo from 'vue-apollo'; +import { GlAlert, GlModal } from '@gitlab/ui'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import { convertToGraphQLId } from '~/graphql_shared/utils'; +import CreateTimelogForm from '~/sidebar/components/time_tracking/create_timelog_form.vue'; +import createTimelogMutation from '~/sidebar/components/time_tracking/graphql/mutations/create_timelog.mutation.graphql'; +import { TYPE_ISSUE, TYPE_MERGE_REQUEST } from '~/graphql_shared/constants'; + +const mockMutationErrorMessage = 'Example error message'; + +const resolvedMutationWithoutErrorsMock = jest.fn().mockResolvedValue({ + data: { + timelogCreate: { + errors: [], + timelog: { + id: 'gid://gitlab/Timelog/1', + issue: {}, + mergeRequest: {}, + }, + }, + }, +}); + +const resolvedMutationWithErrorsMock = jest.fn().mockResolvedValue({ + data: { + timelogCreate: { + errors: [{ message: mockMutationErrorMessage }], + timelog: null, + }, + }, +}); + +const rejectedMutationMock = jest.fn().mockRejectedValue(); +const modalCloseMock = jest.fn(); + +describe('Create Timelog Form', () => { + Vue.use(VueApollo); + + let wrapper; + let fakeApollo; + + const findForm = () => wrapper.find('form'); + const findModal = () => wrapper.findComponent(GlModal); + const findAlert = () => wrapper.findComponent(GlAlert); + const findSaveButton = () => findModal().props('actionPrimary'); + const findSaveButtonLoadingState = () => findSaveButton().attributes[0].loading; + const findSaveButtonDisabledState = () => findSaveButton().attributes[0].disabled; + + const submitForm = () => findForm().trigger('submit'); + + const mountComponent = ( + { props, data, providedProps } = {}, + mutationResolverMock = rejectedMutationMock, + ) => { + fakeApollo = createMockApollo([[createTimelogMutation, mutationResolverMock]]); + + wrapper = shallowMountExtended(CreateTimelogForm, { + data() { + return { + ...data, + }; + }, + provide: { + issuableType: 'issue', + ...providedProps, + }, + propsData: { + issuableId: '1', + ...props, + }, + apolloProvider: fakeApollo, + }); + + wrapper.vm.$refs.modal.close = modalCloseMock; + }; + + afterEach(() => { + wrapper.destroy(); + fakeApollo = null; + }); + + describe('save button', () => { + it('is disabled and not loading by default', () => { + mountComponent(); + + expect(findSaveButtonLoadingState()).toBe(false); + expect(findSaveButtonDisabledState()).toBe(true); + }); + + it('is enabled and not loading when time spent is not empty', () => { + mountComponent({ data: { timeSpent: '2d' } }); + + expect(findSaveButtonLoadingState()).toBe(false); + expect(findSaveButtonDisabledState()).toBe(false); + }); + + it('is disabled and loading when the the form is submitted', async () => { + mountComponent({ data: { timeSpent: '2d' } }); + + submitForm(); + + await nextTick(); + + expect(findSaveButtonLoadingState()).toBe(true); + expect(findSaveButtonDisabledState()).toBe(true); + }); + + it('is enabled and not loading the when form is submitted but the mutation has errors', async () => { + mountComponent({ data: { timeSpent: '2d' } }); + + submitForm(); + + await waitForPromises(); + + expect(rejectedMutationMock).toHaveBeenCalled(); + expect(findSaveButtonLoadingState()).toBe(false); + expect(findSaveButtonDisabledState()).toBe(false); + }); + + it('is enabled and not loading the when form is submitted but the mutation returns errors', async () => { + mountComponent({ data: { timeSpent: '2d' } }, resolvedMutationWithErrorsMock); + + submitForm(); + + await waitForPromises(); + + expect(resolvedMutationWithErrorsMock).toHaveBeenCalled(); + expect(findSaveButtonLoadingState()).toBe(false); + expect(findSaveButtonDisabledState()).toBe(false); + }); + }); + + describe('form', () => { + it('does not call any mutation when the the form is incomplete', async () => { + mountComponent(); + + submitForm(); + + await waitForPromises(); + + expect(rejectedMutationMock).not.toHaveBeenCalled(); + }); + + it('closes the modal after a successful mutation', async () => { + mountComponent({ data: { timeSpent: '2d' } }, resolvedMutationWithoutErrorsMock); + + submitForm(); + + await waitForPromises(); + await nextTick(); + + expect(modalCloseMock).toHaveBeenCalled(); + }); + + it.each` + issuableType | typeConstant + ${'issue'} | ${TYPE_ISSUE} + ${'merge_request'} | ${TYPE_MERGE_REQUEST} + `( + 'calls the mutation with all the fields when the the form is submitted and issuable type is $issuableType', + async ({ issuableType, typeConstant }) => { + const timeSpent = '2d'; + const spentAt = '2022-11-20T21:53:00+0000'; + const summary = 'Example'; + + mountComponent({ data: { timeSpent, spentAt, summary }, providedProps: { issuableType } }); + + submitForm(); + + await waitForPromises(); + + expect(rejectedMutationMock).toHaveBeenCalledWith({ + input: { timeSpent, spentAt, summary, issuableId: convertToGraphQLId(typeConstant, '1') }, + }); + }, + ); + }); + + describe('alert', () => { + it('is hidden by default', () => { + mountComponent(); + + expect(findAlert().exists()).toBe(false); + }); + + it('shows an error if the submission fails with a handled error', async () => { + mountComponent({ data: { timeSpent: '2d' } }, resolvedMutationWithErrorsMock); + + submitForm(); + + await waitForPromises(); + + expect(findAlert().exists()).toBe(true); + expect(findAlert().text()).toBe(mockMutationErrorMessage); + }); + + it('shows an error if the submission fails with an unhandled error', async () => { + mountComponent({ data: { timeSpent: '2d' } }); + + submitForm(); + + await waitForPromises(); + + expect(findAlert().exists()).toBe(true); + expect(findAlert().text()).toBe('An error occurred while saving the time entry.'); + }); + }); +}); diff --git a/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js b/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js index 835e700e63cb91..45d8b5e4647931 100644 --- a/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js +++ b/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js @@ -268,47 +268,32 @@ describe('Issuable Time Tracker', () => { }); }); - describe('Help pane', () => { - const findHelpButton = () => findByTestId('helpButton'); - const findCloseHelpButton = () => findByTestId('closeHelpButton'); - - beforeEach(async () => { - wrapper = mountComponent({ - props: { - initialTimeTracking: { - timeEstimate: 0, - totalTimeSpent: 0, - humanTimeEstimate: '', - humanTotalTimeSpent: '', + describe('Add button', () => { + const findAddButton = () => findByTestId('add-time-entry-button'); + + it.each` + visibility | canAddTimeEntries + ${'not visible'} | ${false} + ${'visible'} | ${true} + `( + 'is $visibility when canAddTimeEntries is $canAddTimeEntries', + async ({ canAddTimeEntries }) => { + wrapper = mountComponent({ + props: { + initialTimeTracking: { + timeEstimate: 0, + totalTimeSpent: 0, + humanTimeEstimate: '', + humanTotalTimeSpent: '', + }, + canAddTimeEntries, }, - }, - }); - await nextTick(); - }); - - it('should not show the "Help" pane by default', () => { - expect(findByTestId('helpPane').exists()).toBe(false); - }); - - it('should show the "Help" pane when help button is clicked', async () => { - findHelpButton().trigger('click'); - - await nextTick(); - - expect(findByTestId('helpPane').exists()).toBe(true); - }); - - it('should not show the "Help" pane when help button is clicked and then closed', async () => { - findHelpButton().trigger('click'); - await nextTick(); - - expect(findByTestId('helpPane').exists()).toBe(true); - - findCloseHelpButton().trigger('click'); - await nextTick(); + }); + await nextTick(); - expect(findByTestId('helpPane').exists()).toBe(false); - }); + expect(findAddButton().exists()).toBe(canAddTimeEntries); + }, + ); }); }); diff --git a/spec/requests/api/graphql/mutations/timelogs/create_spec.rb b/spec/requests/api/graphql/mutations/timelogs/create_spec.rb index eea04b89783bcc..9b89d053901ece 100644 --- a/spec/requests/api/graphql/mutations/timelogs/create_spec.rb +++ b/spec/requests/api/graphql/mutations/timelogs/create_spec.rb @@ -11,14 +11,6 @@ let(:current_user) { nil } let(:users_container) { project } - let(:mutation) do - graphql_mutation(:timelogCreate, { - 'time_spent' => time_spent, - 'spent_at' => '2022-07-08', - 'summary' => 'Test summary', - 'issuable_id' => issuable.to_global_id.to_s - }) - end let(:mutation_response) { graphql_mutation_response(:timelog_create) } diff --git a/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb b/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb index 9e8478d76381e4..db656bab0f4002 100644 --- a/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb +++ b/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb @@ -1,6 +1,17 @@ # frozen_string_literal: true RSpec.shared_examples 'issuable supports timelog creation mutation' do + let(:mutation_response) { graphql_mutation_response(:timelog_create) } + let(:mutation) do + variables = { + 'time_spent' => time_spent, + 'spent_at' => '2022-11-16T12:59:35+0100', + 'summary' => 'Test summary', + 'issuable_id' => issuable.to_global_id.to_s + } + graphql_mutation(:timelogCreate, variables) + end + context 'when the user is anonymous' do before do post_graphql_mutation(mutation, current_user: current_user) @@ -38,7 +49,8 @@ expect(mutation_response['errors']).to be_empty expect(mutation_response['timelog']).to include( 'timeSpent' => 3600, - 'spentAt' => '2022-07-08T00:00:00Z', + # This also checks that the ISO time was converted to UTC + 'spentAt' => '2022-11-16T11:59:35Z', 'summary' => 'Test summary' ) end @@ -61,6 +73,17 @@ end RSpec.shared_examples 'issuable does not support timelog creation mutation' do + let(:mutation_response) { graphql_mutation_response(:timelog_create) } + let(:mutation) do + variables = { + 'time_spent' => time_spent, + 'spent_at' => '2022-11-16T12:59:35+0100', + 'summary' => 'Test summary', + 'issuable_id' => issuable.to_global_id.to_s + } + graphql_mutation(:timelogCreate, variables) + end + context 'when the user is anonymous' do before do post_graphql_mutation(mutation, current_user: current_user) -- GitLab From 81798a4ab4ea92ae24c1e24982381c8899260c94 Mon Sep 17 00:00:00 2001 From: Marco Zille Date: Fri, 25 Nov 2022 23:32:07 +0100 Subject: [PATCH 2/9] Fixed broken tests --- .../time_tracking/create_timelog_form.vue | 1 + ...merge_request_sidebar_basic_entity_spec.rb | 6 ++-- locale/gitlab.pot | 9 +++++ ...e_tracking_quick_action_shared_examples.rb | 36 +++++++------------ 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue b/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue index 7f9140df43a11c..29388e350b3918 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue @@ -132,6 +132,7 @@ export default { :title="__('Add time entry')" modal-id="create-timelog-modal" size="sm" + data-testid="create-timelog-modal" :action-primary="primaryProps" :action-cancel="cancelProps" @primary="registerTimeSpent" diff --git a/ee/spec/serializers/merge_request_sidebar_basic_entity_spec.rb b/ee/spec/serializers/merge_request_sidebar_basic_entity_spec.rb index 3d625bb67c4a2d..b50399f43a6b64 100644 --- a/ee/spec/serializers/merge_request_sidebar_basic_entity_spec.rb +++ b/ee/spec/serializers/merge_request_sidebar_basic_entity_spec.rb @@ -17,7 +17,7 @@ stub_feature_flags(gitlab_employee_badge: false) expect(entity[:current_user].keys).to contain_exactly( - :id, :name, :username, :state, :avatar_url, :web_url, :todo, + :id, :name, :username, :state, :avatar_url, :web_url, :todo, :can_create_timelogs, :can_edit, :can_move, :can_admin_label, :can_merge, :can_update_merge_request ) end @@ -29,7 +29,7 @@ allow(Gitlab).to receive(:com?).and_return(false) expect(entity[:current_user].keys).to contain_exactly( - :id, :name, :username, :state, :avatar_url, :web_url, :todo, + :id, :name, :username, :state, :avatar_url, :web_url, :todo, :can_create_timelogs, :can_edit, :can_move, :can_admin_label, :can_merge, :can_update_merge_request ) end @@ -41,7 +41,7 @@ allow(Gitlab).to receive(:com?).and_return(true) expect(entity[:current_user].keys).to contain_exactly( - :id, :name, :username, :state, :avatar_url, :web_url, :todo, + :id, :name, :username, :state, :avatar_url, :web_url, :todo, :can_create_timelogs, :can_edit, :can_move, :can_admin_label, :can_merge, :is_gitlab_employee, :can_update_merge_request ) end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 26784602b0b5e0..d677a8c8437ad3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1609,6 +1609,9 @@ msgstr "" msgid "409|There was a conflict with your request." msgstr "" +msgid "4h 30m" +msgstr "" + msgid "8 hours" msgstr "" @@ -2383,6 +2386,9 @@ msgstr "" msgid "Add text to the sign-in page. Markdown enabled." msgstr "" +msgid "Add time entry" +msgstr "" + msgid "Add to board" msgstr "" @@ -4506,6 +4512,9 @@ msgid_plural "An error occurred while saving the settings" msgstr[0] "" msgstr[1] "" +msgid "An error occurred while saving the time entry." +msgstr "" + msgid "An error occurred while saving your settings. Try saving them again." msgstr "" diff --git a/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb b/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb index 18304951e41b6a..56a1cee44c877e 100644 --- a/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb @@ -22,6 +22,12 @@ def open_time_tracking_report end end + def open_create_timelog_form + page.within time_tracker_selector do + find('[data-testid="add-time-entry-button"]').click + end + end + it 'renders the sidebar component empty state' do page.within '[data-testid="noTrackingPane"]' do expect(page).to have_content 'No estimate or time spent' @@ -74,11 +80,13 @@ def open_time_tracking_report end end - it 'shows the help state when icon is clicked' do - page.within time_tracker_selector do - find('[data-testid="helpButton"]').click - expect(page).to have_content 'Track time with quick actions' - expect(page).to have_content 'Learn more' + it 'shows the create timelog form when add button is clicked' do + open_create_timelog_form + + page.within '[data-testid="create-timelog-modal"]' do + expect(page).to have_content 'Add time entry' + expect(page).to have_content 'Time spent' + expect(page).to have_content 'Spent at' end end @@ -123,24 +131,6 @@ def open_time_tracking_report expect(page).to have_content '1d' end end - - it 'hides the help state when close icon is clicked' do - page.within time_tracker_selector do - find('[data-testid="helpButton"]').click - find('[data-testid="closeHelpButton"]').click - - expect(page).not_to have_content 'Track time with quick actions' - expect(page).not_to have_content 'Learn more' - end - end - - it 'displays the correct help url' do - page.within time_tracker_selector do - find('[data-testid="helpButton"]').click - - expect(find_link('Learn more')[:href]).to have_content('/help/user/project/time_tracking.md') - end - end end def submit_time(quick_action) -- GitLab From 9cba84944a8aa7d100dabf5b33dc6cbc8e2ca380 Mon Sep 17 00:00:00 2001 From: Marco Zille Date: Tue, 29 Nov 2022 09:56:19 +0100 Subject: [PATCH 3/9] Applied suggestions from review --- .../time_tracking/create_timelog_form.vue | 14 +++++++++----- app/graphql/mutations/timelogs/create.rb | 6 +++++- locale/gitlab.pot | 6 +++--- .../mutations/timelogs/create_shared_examples.rb | 3 ++- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue b/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue index 29388e350b3918..a0467eed3dce78 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue @@ -144,16 +144,20 @@ export default { @submit.prevent="registerTimeSpent" >
- + - +
- - + + {{ saveError }} diff --git a/app/graphql/mutations/timelogs/create.rb b/app/graphql/mutations/timelogs/create.rb index cf4fad56ec9f73..1be023eed8a8b2 100644 --- a/app/graphql/mutations/timelogs/create.rb +++ b/app/graphql/mutations/timelogs/create.rb @@ -28,8 +28,12 @@ class Create < Base authorize :create_timelog def resolve(issuable_id:, time_spent:, spent_at:, summary:, **args) - issuable = authorized_find!(id: issuable_id) parsed_time_spent = Gitlab::TimeTrackingFormatter.parse(time_spent) + if parsed_time_spent.nil? + return { timelog: nil, errors: [_('Time spent must be formatted correctly. For example: 1h 30m.')] } + end + + issuable = authorized_find!(id: issuable_id) result = ::Timelogs::CreateService.new( issuable, parsed_time_spent, spent_at, summary, current_user diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d677a8c8437ad3..b478caa6b5efd9 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1609,9 +1609,6 @@ msgstr "" msgid "409|There was a conflict with your request." msgstr "" -msgid "4h 30m" -msgstr "" - msgid "8 hours" msgstr "" @@ -42541,6 +42538,9 @@ msgstr "" msgid "Time spent" msgstr "" +msgid "Time spent must be formatted correctly. For example: 1h 30m." +msgstr "" + msgid "Time to Restore Service" msgstr "" diff --git a/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb b/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb index db656bab0f4002..c6402a89f02dc2 100644 --- a/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb +++ b/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb @@ -65,7 +65,8 @@ end.to change { Timelog.count }.by(0) expect(response).to have_gitlab_http_status(:success) - expect(mutation_response['errors']).to match_array(['Time spent can\'t be blank']) + expect(mutation_response['errors']).to match_array( + ['Time spent must be formatted correctly. For example: 1h 30m.']) expect(mutation_response['timelog']).to be_nil end end -- GitLab From 2293a04851150a3d56fba2b393a75496ca6578d1 Mon Sep 17 00:00:00 2001 From: Marco Zille Date: Wed, 30 Nov 2022 22:30:58 +0100 Subject: [PATCH 4/9] Fixed conflicts with master --- .../sidebar/components/time_tracking/create_timelog_form.vue | 2 +- locale/gitlab.pot | 3 +++ .../components/time_tracking/create_timelog_form_spec.js | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue b/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue index a0467eed3dce78..c6d7e86ea2eea0 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue @@ -11,7 +11,7 @@ import { convertToGraphQLId } from '~/graphql_shared/utils'; import { formatDate } from '~/lib/utils/datetime_utility'; import { TYPE_ISSUE, TYPE_MERGE_REQUEST } from '~/graphql_shared/constants'; import { __ } from '~/locale'; -import createTimelogMutation from './graphql/mutations/create_timelog.mutation.graphql'; +import createTimelogMutation from '../../queries/create_timelog.mutation.graphql'; export default { components: { diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b478caa6b5efd9..92a3af08067b85 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -16275,6 +16275,9 @@ msgstr "" msgid "Example: (jar|exe)$" msgstr "" +msgid "Example: 1h 30m" +msgstr "" + msgid "Example: @sub\\.company\\.com$" msgstr "" diff --git a/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js b/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js index eb2ea00c0ef003..78f8c360759b76 100644 --- a/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js +++ b/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js @@ -6,7 +6,7 @@ import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import { convertToGraphQLId } from '~/graphql_shared/utils'; import CreateTimelogForm from '~/sidebar/components/time_tracking/create_timelog_form.vue'; -import createTimelogMutation from '~/sidebar/components/time_tracking/graphql/mutations/create_timelog.mutation.graphql'; +import createTimelogMutation from '~/sidebar/queries/create_timelog.mutation.graphql'; import { TYPE_ISSUE, TYPE_MERGE_REQUEST } from '~/graphql_shared/constants'; const mockMutationErrorMessage = 'Example error message'; -- GitLab From c39c204398b5d2dbe874ad9887713e9fcb51e54d Mon Sep 17 00:00:00 2001 From: Marco Zille Date: Fri, 2 Dec 2022 14:55:07 +0100 Subject: [PATCH 5/9] Applied suggestion from review --- .../sidebar/components/time_tracking/create_timelog_form_spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js b/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js index 78f8c360759b76..78e227ef254920 100644 --- a/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js +++ b/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js @@ -78,7 +78,6 @@ describe('Create Timelog Form', () => { }; afterEach(() => { - wrapper.destroy(); fakeApollo = null; }); -- GitLab From be684d98a459aae553418880baf3ee50df245a74 Mon Sep 17 00:00:00 2001 From: Marco Zille Date: Mon, 5 Dec 2022 16:24:04 +0100 Subject: [PATCH 6/9] Applied suggestions from review --- .../time_tracking/create_timelog_form.vue | 45 ++++++++++++++---- .../components/time_tracking/time_tracker.vue | 4 +- app/services/timelogs/base_service.rb | 4 +- app/services/timelogs/create_service.rb | 3 ++ .../services/timelogs/create_service_spec.rb | 6 +-- .../time_tracking/create_timelog_form_spec.js | 9 ++++ spec/services/timelogs/create_service_spec.rb | 6 +-- .../create_service_shared_examples.rb | 47 ++++++++++++++++++- 8 files changed, 100 insertions(+), 24 deletions(-) diff --git a/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue b/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue index c6d7e86ea2eea0..b4cbb1fa6649dc 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/create_timelog_form.vue @@ -6,13 +6,18 @@ import { GlFormTextarea, GlModal, GlAlert, + GlLink, + GlSprintf, } from '@gitlab/ui'; import { convertToGraphQLId } from '~/graphql_shared/utils'; import { formatDate } from '~/lib/utils/datetime_utility'; import { TYPE_ISSUE, TYPE_MERGE_REQUEST } from '~/graphql_shared/constants'; -import { __ } from '~/locale'; +import { joinPaths } from '~/lib/utils/url_utility'; +import { s__ } from '~/locale'; import createTimelogMutation from '../../queries/create_timelog.mutation.graphql'; +export const CREATE_TIMELOG_MODAL_ID = 'create-timelog-modal'; + export default { components: { GlDatepicker, @@ -21,6 +26,8 @@ export default { GlFormTextarea, GlModal, GlAlert, + GlLink, + GlSprintf, }, inject: ['issuableType'], props: { @@ -44,7 +51,7 @@ export default { }, primaryProps() { return { - text: __('Save'), + text: s__('CreateTimelogForm|Save'), attributes: [ { variant: 'confirm', @@ -56,9 +63,12 @@ export default { }, cancelProps() { return { - text: __('Cancel'), + text: s__('CreateTimelogForm|Cancel'), }; }, + timeTrackignDocsPath() { + return joinPaths(gon.relative_url_root || '', '/help/user/project/time_tracking.md'); + }, }, methods: { resetModal() { @@ -89,7 +99,7 @@ export default { input: { timeSpent: this.timeSpent, spentAt: this.spentAt - ? formatDate(this.spentAt, 'isoDateTime', true) + ? formatDate(this.spentAt, 'isoDateTime') : formatDate(Date.now(), 'isoDateTime'), summary: this.summary, issuableId: this.getIssuableId(), @@ -104,7 +114,9 @@ export default { } }) .catch((error) => { - this.saveError = error?.message || __('An error occurred while saving the time entry.'); + this.saveError = + error?.message || + s__('CreateTimelogForm|An error occurred while saving the time entry.'); }) .finally(() => { this.isLoading = false; @@ -129,8 +141,8 @@ export default {