From 2e1b4b8953de735556cfb94f61168a01ac41a8ef Mon Sep 17 00:00:00 2001
From: Eugenia Grieff <egrieff@gitlab.com>
Date: Tue, 28 Jun 2022 15:41:53 +0000
Subject: [PATCH] Move description widget update to service layer
- Add services for description widget
---
.../mutations/work_items/update_arguments.rb | 3 ++
app/graphql/mutations/work_items/update.rb | 12 +++++
.../mutations/work_items/update_widgets.rb | 1 +
app/models/work_items/widgets/description.rb | 4 --
app/services/work_items/update_service.rb | 15 +++++-
.../work_items/widgets/base_service.rb | 14 +++++
.../description_service/update_service.rb | 15 ++++++
doc/api/graphql/reference/index.md | 2 +
.../widgets/description_input_type_spec.rb | 9 ++++
.../mutations/work_items/update_spec.rb | 35 ++++++++++++-
.../work_items/update_widgets_spec.rb | 51 +++++++------------
.../update_service_spec.rb | 35 +++++++++++++
...date_description_widget_shared_examples.rb | 34 +++++++++++++
13 files changed, 190 insertions(+), 40 deletions(-)
create mode 100644 app/services/work_items/widgets/base_service.rb
create mode 100644 app/services/work_items/widgets/description_service/update_service.rb
create mode 100644 spec/graphql/types/work_items/widgets/description_input_type_spec.rb
create mode 100644 spec/services/work_items/widgets/description_service/update_service_spec.rb
create mode 100644 spec/support/shared_examples/graphql/mutations/work_items/update_description_widget_shared_examples.rb
diff --git a/app/graphql/mutations/concerns/mutations/work_items/update_arguments.rb b/app/graphql/mutations/concerns/mutations/work_items/update_arguments.rb
index 6a91a097a177..7af55d42f2af 100644
--- a/app/graphql/mutations/concerns/mutations/work_items/update_arguments.rb
+++ b/app/graphql/mutations/concerns/mutations/work_items/update_arguments.rb
@@ -15,6 +15,9 @@ module UpdateArguments
argument :title, GraphQL::Types::String,
required: false,
description: copy_field_description(Types::WorkItemType, :title)
+ argument :description_widget, ::Types::WorkItems::Widgets::DescriptionInputType,
+ required: false,
+ description: 'Input for description widget.'
end
end
end
diff --git a/app/graphql/mutations/work_items/update.rb b/app/graphql/mutations/work_items/update.rb
index c495da00f41e..ff4aba4830f1 100644
--- a/app/graphql/mutations/work_items/update.rb
+++ b/app/graphql/mutations/work_items/update.rb
@@ -24,11 +24,13 @@ def resolve(id:, **attributes)
end
spam_params = ::Spam::SpamParams.new_from_request(request: context[:request])
+ widget_params = extract_widget_params(work_item, attributes)
::WorkItems::UpdateService.new(
project: work_item.project,
current_user: current_user,
params: attributes,
+ widget_params: widget_params,
spam_params: spam_params
).execute(work_item)
@@ -45,6 +47,16 @@ def resolve(id:, **attributes)
def find_object(id:)
GitlabSchema.find_by_gid(id)
end
+
+ def extract_widget_params(work_item, attributes)
+ # Get the list of widgets for the work item's type to extract only the supported attributes
+ widget_keys = work_item.work_item_type.widgets.map(&:api_symbol)
+ widget_params = attributes.extract!(*widget_keys)
+
+ # Cannot use prepare to use `.to_h` on each input due to
+ # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87472#note_945199865
+ widget_params.transform_values { |values| values.to_h }
+ end
end
end
end
diff --git a/app/graphql/mutations/work_items/update_widgets.rb b/app/graphql/mutations/work_items/update_widgets.rb
index d19da0abaacb..7037b7e5a2a6 100644
--- a/app/graphql/mutations/work_items/update_widgets.rb
+++ b/app/graphql/mutations/work_items/update_widgets.rb
@@ -2,6 +2,7 @@
module Mutations
module WorkItems
+ # TODO: Deprecate in favor of using WorkItemUpdate. See https://gitlab.com/gitlab-org/gitlab/-/issues/366300
class UpdateWidgets < BaseMutation
graphql_name 'WorkItemUpdateWidgets'
description "Updates the attributes of a work item's widgets by global ID." \
diff --git a/app/models/work_items/widgets/description.rb b/app/models/work_items/widgets/description.rb
index 35b6d295321c..1e84d172befb 100644
--- a/app/models/work_items/widgets/description.rb
+++ b/app/models/work_items/widgets/description.rb
@@ -4,10 +4,6 @@ module WorkItems
module Widgets
class Description < Base
delegate :description, to: :work_item
-
- def update(params:)
- work_item.description = params[:description] if params&.key?(:description)
- end
end
end
end
diff --git a/app/services/work_items/update_service.rb b/app/services/work_items/update_service.rb
index 0b420881b4bb..7b50040a7162 100644
--- a/app/services/work_items/update_service.rb
+++ b/app/services/work_items/update_service.rb
@@ -6,6 +6,7 @@ def initialize(project:, current_user: nil, params: {}, spam_params: nil, widget
super(project: project, current_user: current_user, params: params, spam_params: nil)
@widget_params = widget_params
+ @widget_services = {}
end
private
@@ -24,8 +25,20 @@ def after_update(work_item)
def execute_widgets(work_item:, callback:)
work_item.widgets.each do |widget|
- widget.try(callback, params: @widget_params[widget.class.api_symbol])
+ widget_service(widget).try(callback, params: @widget_params[widget.class.api_symbol])
end
end
+
+ def widget_service(widget)
+ service_class = begin
+ "WorkItems::Widgets::#{widget.type.capitalize}Service::UpdateService".constantize
+ rescue NameError
+ nil
+ end
+
+ return unless service_class
+
+ @widget_services[widget] ||= service_class.new(widget: widget, current_user: current_user)
+ end
end
end
diff --git a/app/services/work_items/widgets/base_service.rb b/app/services/work_items/widgets/base_service.rb
new file mode 100644
index 000000000000..72debc272bd0
--- /dev/null
+++ b/app/services/work_items/widgets/base_service.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module WorkItems
+ module Widgets
+ class BaseService < ::BaseService
+ attr_reader :widget, :current_user
+
+ def initialize(widget:, current_user:)
+ @widget = widget
+ @current_user = current_user
+ end
+ end
+ end
+end
diff --git a/app/services/work_items/widgets/description_service/update_service.rb b/app/services/work_items/widgets/description_service/update_service.rb
new file mode 100644
index 000000000000..e63b6b2ee6c1
--- /dev/null
+++ b/app/services/work_items/widgets/description_service/update_service.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module WorkItems
+ module Widgets
+ module DescriptionService
+ class UpdateService < WorkItems::Widgets::BaseService
+ def update(params: {})
+ return unless params.present? && params[:description]
+
+ widget.work_item.description = params[:description]
+ end
+ end
+ end
+ end
+end
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index af91c842838e..c2d62421425e 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -5608,6 +5608,7 @@ Input type: `WorkItemUpdateInput`
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="mutationworkitemupdateclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationworkitemupdatedescriptionwidget"></a>`descriptionWidget` | [`WorkItemWidgetDescriptionInput`](#workitemwidgetdescriptioninput) | Input for description widget. |
| <a id="mutationworkitemupdateid"></a>`id` | [`WorkItemID!`](#workitemid) | Global ID of the work item. |
| <a id="mutationworkitemupdatestateevent"></a>`stateEvent` | [`WorkItemStateEvent`](#workitemstateevent) | Close or reopen a work item. |
| <a id="mutationworkitemupdatetitle"></a>`title` | [`String`](#string) | Title of the work item. |
@@ -21925,6 +21926,7 @@ A time-frame defined as a closed inclusive range of two dates.
| Name | Type | Description |
| ---- | ---- | ----------- |
+| <a id="workitemupdatedtaskinputdescriptionwidget"></a>`descriptionWidget` | [`WorkItemWidgetDescriptionInput`](#workitemwidgetdescriptioninput) | Input for description widget. |
| <a id="workitemupdatedtaskinputid"></a>`id` | [`WorkItemID!`](#workitemid) | Global ID of the work item. |
| <a id="workitemupdatedtaskinputstateevent"></a>`stateEvent` | [`WorkItemStateEvent`](#workitemstateevent) | Close or reopen a work item. |
| <a id="workitemupdatedtaskinputtitle"></a>`title` | [`String`](#string) | Title of the work item. |
diff --git a/spec/graphql/types/work_items/widgets/description_input_type_spec.rb b/spec/graphql/types/work_items/widgets/description_input_type_spec.rb
new file mode 100644
index 000000000000..81c64bc38ab4
--- /dev/null
+++ b/spec/graphql/types/work_items/widgets/description_input_type_spec.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Types::WorkItems::Widgets::DescriptionInputType do
+ it { expect(described_class.graphql_name).to eq('WorkItemWidgetDescriptionInput') }
+
+ it { expect(described_class.arguments.keys).to match_array(%w[description]) }
+end
diff --git a/spec/requests/api/graphql/mutations/work_items/update_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_spec.rb
index 71b03103115f..7a160819a418 100644
--- a/spec/requests/api/graphql/mutations/work_items/update_spec.rb
+++ b/spec/requests/api/graphql/mutations/work_items/update_spec.rb
@@ -11,8 +11,17 @@
let(:work_item_event) { 'CLOSE' }
let(:input) { { 'stateEvent' => work_item_event, 'title' => 'updated title' } }
+ let(:fields) do
+ <<~FIELDS
+ workItem {
+ state
+ title
+ }
+ errors
+ FIELDS
+ end
- let(:mutation) { graphql_mutation(:workItemUpdate, input.merge('id' => work_item.to_global_id.to_s)) }
+ let(:mutation) { graphql_mutation(:workItemUpdate, input.merge('id' => work_item.to_global_id.to_s), fields) }
let(:mutation_response) { graphql_mutation_response(:work_item_update) }
@@ -80,5 +89,29 @@
expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project')
end
end
+
+ context 'with description widget input' do
+ let(:fields) do
+ <<~FIELDS
+ workItem {
+ description
+ widgets {
+ type
+ ... on WorkItemWidgetDescription {
+ description
+ }
+ }
+ }
+ errors
+ FIELDS
+ end
+
+ it_behaves_like 'update work item description widget' do
+ let(:new_description) { 'updated description' }
+ let(:input) do
+ { 'descriptionWidget' => { 'description' => new_description } }
+ end
+ end
+ end
end
end
diff --git a/spec/requests/api/graphql/mutations/work_items/update_widgets_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_widgets_spec.rb
index 595d8fe97ede..2a5cb937a2f9 100644
--- a/spec/requests/api/graphql/mutations/work_items/update_widgets_spec.rb
+++ b/spec/requests/api/graphql/mutations/work_items/update_widgets_spec.rb
@@ -9,16 +9,23 @@
let_it_be(:developer) { create(:user).tap { |user| project.add_developer(user) } }
let_it_be(:work_item, refind: true) { create(:work_item, project: project) }
- let(:input) do
- {
- 'descriptionWidget' => { 'description' => 'updated description' }
+ let(:input) { { 'descriptionWidget' => { 'description' => 'updated description' } } }
+ let(:mutation_response) { graphql_mutation_response(:work_item_update_widgets) }
+ let(:mutation) do
+ graphql_mutation(:workItemUpdateWidgets, input.merge('id' => work_item.to_global_id.to_s), <<~FIELDS)
+ errors
+ workItem {
+ description
+ widgets {
+ type
+ ... on WorkItemWidgetDescription {
+ description
+ }
+ }
}
+ FIELDS
end
- let(:mutation) { graphql_mutation(:workItemUpdateWidgets, input.merge('id' => work_item.to_global_id.to_s)) }
-
- let(:mutation_response) { graphql_mutation_response(:work_item_update_widgets) }
-
context 'the user is not allowed to update a work item' do
let(:current_user) { create(:user) }
@@ -28,32 +35,8 @@
context 'when user has permissions to update a work item', :aggregate_failures do
let(:current_user) { developer }
- context 'when the updated work item is not valid' do
- it 'returns validation errors without the work item' do
- errors = ActiveModel::Errors.new(work_item).tap { |e| e.add(:description, 'error message') }
-
- allow_next_found_instance_of(::WorkItem) do |instance|
- allow(instance).to receive(:valid?).and_return(false)
- allow(instance).to receive(:errors).and_return(errors)
- end
-
- post_graphql_mutation(mutation, current_user: current_user)
-
- expect(mutation_response['workItem']).to be_nil
- expect(mutation_response['errors']).to match_array(['Description error message'])
- end
- end
-
- it 'updates the work item widgets' do
- expect do
- post_graphql_mutation(mutation, current_user: current_user)
- work_item.reload
- end.to change(work_item, :description).from(nil).to('updated description')
-
- expect(response).to have_gitlab_http_status(:success)
- expect(mutation_response['workItem']).to include(
- 'title' => work_item.title
- )
+ it_behaves_like 'update work item description widget' do
+ let(:new_description) { 'updated description' }
end
it_behaves_like 'has spam protection' do
@@ -69,7 +52,7 @@
expect do
post_graphql_mutation(mutation, current_user: current_user)
work_item.reload
- end.to not_change(work_item, :title)
+ end.to not_change(work_item, :description)
expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project')
end
diff --git a/spec/services/work_items/widgets/description_service/update_service_spec.rb b/spec/services/work_items/widgets/description_service/update_service_spec.rb
new file mode 100644
index 000000000000..a2eceb97f096
--- /dev/null
+++ b/spec/services/work_items/widgets/description_service/update_service_spec.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe WorkItems::Widgets::DescriptionService::UpdateService do
+ let_it_be(:user) { create(:user) }
+ let_it_be(:project) { create(:project) }
+ let_it_be_with_reload(:work_item) { create(:work_item, project: project, description: 'old description') }
+
+ let(:widget) { work_item.widgets.find {|widget| widget.is_a?(WorkItems::Widgets::Description) } }
+
+ describe '#update' do
+ subject { described_class.new(widget: widget, current_user: user).update(params: params) } # rubocop:disable Rails/SaveBang
+
+ context 'when description param is present' do
+ let(:params) { { description: 'updated description' } }
+
+ it 'correctly sets work item description value' do
+ subject
+
+ expect(work_item.description).to eq('updated description')
+ end
+ end
+
+ context 'when description param is not present' do
+ let(:params) { {} }
+
+ it 'does not change work item description value' do
+ subject
+
+ expect(work_item.description).to eq('old description')
+ end
+ end
+ end
+end
diff --git a/spec/support/shared_examples/graphql/mutations/work_items/update_description_widget_shared_examples.rb b/spec/support/shared_examples/graphql/mutations/work_items/update_description_widget_shared_examples.rb
new file mode 100644
index 000000000000..56c2ca22e15c
--- /dev/null
+++ b/spec/support/shared_examples/graphql/mutations/work_items/update_description_widget_shared_examples.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+RSpec.shared_examples 'update work item description widget' do
+ it 'updates the description widget' do
+ expect do
+ post_graphql_mutation(mutation, current_user: current_user)
+ work_item.reload
+ end.to change(work_item, :description).from(nil).to(new_description)
+
+ expect(response).to have_gitlab_http_status(:success)
+ expect(mutation_response['workItem']['widgets']).to include(
+ {
+ 'description' => new_description,
+ 'type' => 'DESCRIPTION'
+ }
+ )
+ end
+
+ context 'when the updated work item is not valid' do
+ it 'returns validation errors without the work item' do
+ errors = ActiveModel::Errors.new(work_item).tap { |e| e.add(:description, 'error message') }
+
+ allow_next_found_instance_of(::WorkItem) do |instance|
+ allow(instance).to receive(:valid?).and_return(false)
+ allow(instance).to receive(:errors).and_return(errors)
+ end
+
+ post_graphql_mutation(mutation, current_user: current_user)
+
+ expect(mutation_response['workItem']).to be_nil
+ expect(mutation_response['errors']).to match_array(['Description error message'])
+ end
+ end
+end
--
GitLab