diff --git a/app/controllers/groups/saved_views_controller.rb b/app/controllers/groups/saved_views_controller.rb index 266a2f893e255b83d2bf2edaba68ce7f5aeb301d..7014af27470d5011714b15d4ebb87cf707d74bd4 100644 --- a/app/controllers/groups/saved_views_controller.rb +++ b/app/controllers/groups/saved_views_controller.rb @@ -7,7 +7,19 @@ class SavedViewsController < Groups::ApplicationController feature_category :portfolio_management def subscribe - not_found + return not_found unless group.work_items_saved_views_enabled?(current_user) + + # Note: we skip authorization here, since we have logic on the frontend to show an appropriate warning modal if + # the user does not have permission to read the saved view. We only use the ID here to check that it exists at all + view = ::WorkItems::SavedViews::SavedViewsFinder.new( + user: current_user, + namespace: group, + params: { id: params.permit(:id)[:id], skip_authorization: true } + ).execute.first + + return not_found unless view + + redirect_to group_work_items_path(group, saved_view_id: view.id) end end end diff --git a/app/controllers/projects/saved_views_controller.rb b/app/controllers/projects/saved_views_controller.rb index a87d462771b81b4073e6a80bc616c7d2041829fd..3ce58c3827175c26ce001bd7c325455154a19780 100644 --- a/app/controllers/projects/saved_views_controller.rb +++ b/app/controllers/projects/saved_views_controller.rb @@ -7,7 +7,19 @@ class SavedViewsController < Projects::ApplicationController feature_category :portfolio_management def subscribe - not_found + return not_found unless project.work_items_saved_views_enabled?(current_user) + + # Note: we skip authorization here, since we have logic on the frontend to show an appropriate warning modal if + # the user does not have permission to read the saved view. We only use the ID here to check that it exists at all + view = ::WorkItems::SavedViews::SavedViewsFinder.new( + user: current_user, + namespace: project.project_namespace, + params: { id: params.permit(:id)[:id], skip_authorization: true } + ).execute.first + + return not_found unless view + + redirect_to project_work_items_path(project, saved_view_id: view.id) end end end diff --git a/app/finders/work_items/saved_views/saved_views_finder.rb b/app/finders/work_items/saved_views/saved_views_finder.rb new file mode 100644 index 0000000000000000000000000000000000000000..9dadf1631de100d6fa1239ebc51b61002ebc0636 --- /dev/null +++ b/app/finders/work_items/saved_views/saved_views_finder.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module WorkItems + module SavedViews + class SavedViewsFinder + attr_reader :user, :namespace, :params + + def initialize(user:, namespace:, params: {}) + @user = user + @namespace = namespace + @params = params + end + + def execute + items = WorkItems::SavedViews::SavedView.in_namespace(namespace) + items = by_visibility(items) + items = by_id(items) + items = by_subscribed(items) + items = by_search(items) + + sort(items) + end + + private + + def by_visibility(items) + return items if params[:skip_authorization] + + items.visible_to(user) + end + + def by_id(items) + params[:id] ? items.id_in(params[:id]) : items + end + + def by_subscribed(items) + return items unless params[:subscribed_only] + + items.subscribed_by(user) + end + + def by_search(items) + params[:search] ? items.search(params[:search]) : items + end + + def sort(items) + items.sort_by_attribute(params[:sort], user: user, scoped_to_subscribed: params[:subscribed_only]) + end + end + end +end diff --git a/app/graphql/mutations/work_items/saved_views/create.rb b/app/graphql/mutations/work_items/saved_views/create.rb index 7b9c2126febb2974e026b3abc9321b8eaab12542..4c662897bb6b6ae178acf3ff34036ffb48912425 100644 --- a/app/graphql/mutations/work_items/saved_views/create.rb +++ b/app/graphql/mutations/work_items/saved_views/create.rb @@ -62,10 +62,21 @@ class Create < BaseMutation scopes: [:api], description: 'Errors encountered during the mutation.' - def resolve(namespace_path:, **_attrs) - authorized_find!(namespace_path) + def resolve(namespace_path:, **attrs) + container = authorized_find!(namespace_path) - { saved_view: nil, errors: [] } + result = ::WorkItems::SavedViews::CreateService.new( + current_user: current_user, + container: container, + params: attrs + ).execute + + if result.success? + check_spam_action_response!(result.payload[:saved_view]) + { saved_view: result.payload[:saved_view], errors: [] } + else + { saved_view: nil, errors: result.errors } + end end end end diff --git a/app/graphql/mutations/work_items/saved_views/delete.rb b/app/graphql/mutations/work_items/saved_views/delete.rb index dc1170c96efe2799754af601da4a151735a1c171..d33e7e295a65ab57d7bdec82f4758205a6ac6be5 100644 --- a/app/graphql/mutations/work_items/saved_views/delete.rb +++ b/app/graphql/mutations/work_items/saved_views/delete.rb @@ -28,9 +28,21 @@ class Delete < BaseMutation description: 'Errors encountered during the mutation.' def resolve(id:) - authorized_find!(id: id) + saved_view = authorized_find!(id: id) - { saved_view: nil, errors: [] } + unless saved_view.namespace.owner_entity.work_items_saved_views_enabled?(current_user) + return { saved_view: nil, errors: ['Saved views are not enabled for this namespace.'] } + end + + saved_view.destroy! + + { saved_view: saved_view, errors: [] } + end + + private + + def find_object(id:) + GitlabSchema.object_from_id(id, expected_type: ::WorkItems::SavedViews::SavedView) end end end diff --git a/app/graphql/mutations/work_items/saved_views/reorder.rb b/app/graphql/mutations/work_items/saved_views/reorder.rb index 2224e80afc47b35d342f8c585c36cd2220dbe026..70b2862eb4ef739c3955973e359a868e83bbae13 100644 --- a/app/graphql/mutations/work_items/saved_views/reorder.rb +++ b/app/graphql/mutations/work_items/saved_views/reorder.rb @@ -19,13 +19,13 @@ class Reorder < BaseMutation ::Types::GlobalIDType[::WorkItems::SavedViews::SavedView], required: false, description: 'Global ID of a saved view that should be placed before the saved view.', - prepare: ->(id, _ctx) { GitlabSchema.parse_gid(id).model_id } + prepare: ->(id, _ctx) { GitlabSchema.parse_gid(id).model_id.to_i } argument :move_after_id, ::Types::GlobalIDType[::WorkItems::SavedViews::SavedView], required: false, description: 'Global ID of a saved view that should be placed after the saved view.', - prepare: ->(id, _ctx) { GitlabSchema.parse_gid(id).model_id } + prepare: ->(id, _ctx) { GitlabSchema.parse_gid(id).model_id.to_i } field :saved_view, ::Types::WorkItems::SavedViews::SavedViewType, @@ -41,10 +41,23 @@ class Reorder < BaseMutation validates mutually_exclusive: [:move_before_id, :move_after_id] - def resolve(id:, **_args) - authorized_find!(id: id) + def resolve(id:, **args) + saved_view = authorized_find!(id: id) - { saved_view: nil, errors: [] } + unless saved_view.namespace.owner_entity.work_items_saved_views_enabled?(current_user) + return { saved_view: nil, errors: ['Saved views are not enabled for this namespace.'] } + end + + result = ::WorkItems::SavedViews::ReorderService.new( + current_user: current_user, + params: args.slice(:move_before_id, :move_after_id) + ).execute(saved_view) + + if result.success? + { saved_view: result.payload[:saved_view].saved_view, errors: [] } + else + { saved_view: nil, errors: [result.message] } + end end end end diff --git a/app/graphql/mutations/work_items/saved_views/subscribe.rb b/app/graphql/mutations/work_items/saved_views/subscribe.rb index b7bd9aede8d52b9b248d21b462e02d7c0c29a5a6..8734226bfeb62c9960c8c17cf05a0a56489bd6c2 100644 --- a/app/graphql/mutations/work_items/saved_views/subscribe.rb +++ b/app/graphql/mutations/work_items/saved_views/subscribe.rb @@ -28,9 +28,19 @@ class Subscribe < BaseMutation description: 'Errors encountered during the mutation.' def resolve(id:) - authorized_find!(id: id) + saved_view = authorized_find!(id: id) - { saved_view: nil, errors: [] } + unless saved_view.namespace.owner_entity.work_items_saved_views_enabled?(current_user) + return { saved_view: nil, errors: ['Saved views are not enabled for this namespace.'] } + end + + res = ::WorkItems::SavedViews::UserSavedView.subscribe(user: current_user, saved_view: saved_view) + + if res + { saved_view: saved_view, errors: [] } + else + { saved_view: nil, errors: ['Saved view limit exceeded.'] } + end end end end diff --git a/app/graphql/mutations/work_items/saved_views/unsubscribe.rb b/app/graphql/mutations/work_items/saved_views/unsubscribe.rb index a7b3bb383b3dcd1c88468577492dc32191e90018..eaa69565d0bf4ce8ff281f72ad010687dcfca54e 100644 --- a/app/graphql/mutations/work_items/saved_views/unsubscribe.rb +++ b/app/graphql/mutations/work_items/saved_views/unsubscribe.rb @@ -22,9 +22,15 @@ class Unsubscribe < BaseMutation description: 'Unsubscribed saved view.' def resolve(id:) - authorized_find!(id: id) + saved_view = authorized_find!(id: id) - { saved_view: nil } + unless saved_view.namespace.owner_entity.work_items_saved_views_enabled?(current_user) + return { saved_view: nil, errors: ['Saved views are not enabled for this namespace.'] } + end + + ::WorkItems::SavedViews::UserSavedView.unsubscribe(user: current_user, saved_view: saved_view) + + { saved_view: saved_view } end end end diff --git a/app/graphql/mutations/work_items/saved_views/update.rb b/app/graphql/mutations/work_items/saved_views/update.rb index 9d4086e6921e40f48dbefaed6f3784a8372ad685..a1ad1e247748fcb091605a69a7e295db4ff67dfa 100644 --- a/app/graphql/mutations/work_items/saved_views/update.rb +++ b/app/graphql/mutations/work_items/saved_views/update.rb @@ -6,10 +6,12 @@ module SavedViews class Update < BaseMutation graphql_name 'WorkItemSavedViewUpdate' - authorize :update_saved_view - description "Updates a saved view." + include Mutations::SpamProtection + + authorize :update_saved_view + argument :id, ::Types::GlobalIDType[::WorkItems::SavedViews::SavedView], required: true, @@ -59,10 +61,27 @@ class Update < BaseMutation scopes: [:api], description: 'Errors encountered during the mutation.' - def resolve(id:, **_attrs) - authorized_find!(id: id) + def resolve(id:, **attrs) + saved_view = authorized_find!(id: id) + + result = ::WorkItems::SavedViews::UpdateService.new( + current_user: current_user, + saved_view: saved_view, + params: attrs + ).execute + + if result.success? + check_spam_action_response!(saved_view) + { saved_view: saved_view, errors: [] } + else + { saved_view: nil, errors: result.message } + end + end + + private - { saved_view: nil, errors: [] } + def find_object(id:) + GitlabSchema.object_from_id(id, expected_type: ::WorkItems::SavedViews::SavedView) end end end diff --git a/app/graphql/resolvers/work_items/saved_views/saved_views_resolver.rb b/app/graphql/resolvers/work_items/saved_views/saved_views_resolver.rb index d93494f657606f12944ae9b051d08630dad97b18..95ce7718a70efd32d747d41f16383b4d43f85fe0 100644 --- a/app/graphql/resolvers/work_items/saved_views/saved_views_resolver.rb +++ b/app/graphql/resolvers/work_items/saved_views/saved_views_resolver.rb @@ -27,8 +27,10 @@ class SavedViewsResolver < BaseResolver type Types::WorkItems::SavedViews::SavedViewType.connection_type, null: true - def resolve(**_args) - ::WorkItems::SavedViews::SavedView.none + def resolve(**args) + ::WorkItems::SavedViews::SavedViewsFinder.new(user: current_user, namespace: object, params: args) + .execute + .preload_namespace end end end diff --git a/app/graphql/types/work_items/saved_views/saved_view_type.rb b/app/graphql/types/work_items/saved_views/saved_view_type.rb index 5941f8b297df73949cf76f64a28f42484b8b4ab1..8c3fbe682d4a847f1e3390c77ed94a0e0183ca87 100644 --- a/app/graphql/types/work_items/saved_views/saved_view_type.rb +++ b/app/graphql/types/work_items/saved_views/saved_view_type.rb @@ -65,21 +65,43 @@ class SavedViewType < BaseObject description: 'Work items associated with the saved view.' def filters - {} + validated_result[:filters] end def filter_warnings - [] + validated_result[:warnings] end def share_url namespace = object.namespace if namespace.is_a?(::Group) - Gitlab::Routing.url_helpers.subscribe_group_saved_view_url(namespace, object.id) + Gitlab::Routing.url_helpers.group_saved_view_url(namespace, object.id) else project = namespace.project - Gitlab::Routing.url_helpers.subscribe_project_saved_view_url(project, object.id) + Gitlab::Routing.url_helpers.project_saved_view_url(project, object.id) + end + end + + def display_settings + return unless object.display_settings + + object.display_settings.except('version') + end + + def sort + object.sort&.to_sym + end + + private + + def validated_result + @validated_result ||= begin + result = ::WorkItems::SavedViews::FilterValidatorService.new( + filter_data: object.filter_data, + namespace: object.namespace, + current_user: current_user).execute + result.payload end end end diff --git a/app/models/work_items/saved_views/saved_view.rb b/app/models/work_items/saved_views/saved_view.rb index 8467fe5905515f8ed3d4197059f0466352d2afb2..667d3c7eb9f6e8e3282b0ff2901911869670237e 100644 --- a/app/models/work_items/saved_views/saved_view.rb +++ b/app/models/work_items/saved_views/saved_view.rb @@ -3,17 +3,69 @@ module WorkItems module SavedViews class SavedView < ApplicationRecord + include Gitlab::SQL::Pattern + include Spammable + belongs_to :namespace belongs_to :author, foreign_key: :created_by_id, optional: true, inverse_of: :created_saved_views, class_name: 'User' - - has_many :user_saved_views, class_name: 'WorkItems::SavedViews::UserSavedView', inverse_of: :saved_view + has_many :user_saved_views, class_name: 'WorkItems::SavedViews::UserSavedView' has_many :subscribed_users, through: :user_saved_views, source: :user - validates :name, presence: true, length: { maximum: 140 } - validates :description, length: { maximum: 140 }, allow_blank: true - validates :version, presence: true, numericality: { greater_than: 0 } - validates :private, inclusion: { in: [true, false] } + scope :in_namespace, ->(namespace) { where(namespace_id: namespace.id) } + scope :authored_by, ->(user) { where(author: user) } + scope :private_only, -> { where(private: true) } + scope :public_only, -> { where(private: false) } + scope :visible_to, ->(user) { user ? private_only.authored_by(user).or(public_only) : public_only } + scope :subscribed_by, ->(user) { joins(:user_saved_views).where(user_saved_views: { user_id: user.id }) } + scope :preload_namespace, -> { preload(:namespace) } + + validates :name, presence: true, length: { maximum: 255 }, uniqueness: { scope: [:namespace_id] } + validates :filter_data, json_schema: { filename: "saved_view_filters", size_limit: 8.kilobytes } + validates :display_settings, json_schema: { filename: "saved_view_display_settings", size_limit: 8.kilobytes } + + attr_spammable :name, spam_title: true + attr_spammable :description, spam_description: true + + enum :sort, ::WorkItems::SortingKeys.all.keys + + scope :order_relative_position, ->(user) { + relation = joins(:user_saved_views) + .where(user_saved_views: { user_id: user.id }) + .order(Arel.sql('user_saved_views.relative_position ASC NULLS LAST'), id: :desc) + + order_object = Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'user_saved_views.relative_position', + column_expression: UserSavedView.arel_table[:relative_position], + order_expression: UserSavedView.arel_table[:relative_position].asc.nulls_last, + nullable: :nulls_last + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + order_expression: SavedView.arel_table[:id].desc + ) + ]) + + relation.reorder(order_object) + } + + def self.sort_by_attribute(method, user: nil, scoped_to_subscribed: false) + # Relative position sorting is only needed for a logged-in user is viewing their subscribed saved views, which + # is restricted to WorkItems::SavedViews::UserSavedView.user_saved_view_limit items. Since this sort can be + # expensive since we need to join then sort, we restrict it to only this case, and fallback to id otherwise + return order_relative_position(user) if method.to_s == 'relative_position' && user && scoped_to_subscribed + + order(id: :desc) + end + + def self.search(query) + fuzzy_search(query, [:name, :description]) + end + + def unsubscribe_other_users!(user:) + user_saved_views.where.not(user: user).delete_all + end end end end diff --git a/app/models/work_items/saved_views/user_saved_view.rb b/app/models/work_items/saved_views/user_saved_view.rb index 2605cacbf539402c289a274cc09d2cbb83ade546..7e0e57360ebab3ce068d30222a497fdcaa7464b4 100644 --- a/app/models/work_items/saved_views/user_saved_view.rb +++ b/app/models/work_items/saved_views/user_saved_view.rb @@ -3,18 +3,55 @@ module WorkItems module SavedViews class UserSavedView < ApplicationRecord + include RelativePositioning + belongs_to :namespace belongs_to :user, inverse_of: :user_saved_views belongs_to :saved_view, class_name: 'WorkItems::SavedViews::SavedView', inverse_of: :user_saved_views validates :saved_view_id, uniqueness: { scope: :user_id } + scope :in_namespace, ->(namespace) { where(namespace: namespace) } + scope :for_user, ->(user) { where(user: user) } + scope :for_saved_view, ->(saved_view) { where(saved_view: saved_view) } + + before_create :set_initial_position + class << self + def subscribe(user:, saved_view:) + namespace = saved_view.namespace + if UserSavedView.where(user: user, namespace: namespace).count >= user_saved_view_limit(namespace) + return false + end + + find_or_create_by!(user: user, saved_view: saved_view, namespace: saved_view.namespace) + + true + end + + def unsubscribe(user:, saved_view:) + find_by(user: user, saved_view: saved_view, namespace: saved_view.namespace)&.destroy + end + + def relative_positioning_query_base(user_saved_view) + where(namespace: user_saved_view.namespace, user: user_saved_view.user) + end + + def relative_positioning_parent_column + :namespace_id + end + # Overridden in EE def user_saved_view_limit(_namespace) 5 end end + + private + + def set_initial_position + move_to_end if relative_position.nil? + end end end end diff --git a/app/models/work_items/widget_definition.rb b/app/models/work_items/widget_definition.rb index c35ae3fd7aecf473c59a0a2d7b786ab3962f0c01..4c1f5e7bec1c19ed3be1a58d43b8a55adb5dc12b 100644 --- a/app/models/work_items/widget_definition.rb +++ b/app/models/work_items/widget_definition.rb @@ -17,6 +17,10 @@ class WidgetDefinition < ApplicationRecord scope :enabled, -> { where(disabled: false) } scope :by_enabled_widget_type, ->(widget_type) { enabled.where(widget_type: widget_type) } + # IMPORTANT: When adding widget types to this enum, they MUST be added to the end of this list. + # The saved views feature stores sort options as integers that map to positions in the list returned + # by WidgetDefinition.widget_classes (which iterates this enum). Inserting widgets in the middle + # would corrupt existing saved view sort values. enum :widget_type, { assignees: 0, description: 1, diff --git a/app/policies/work_items/saved_views/saved_view_policy.rb b/app/policies/work_items/saved_views/saved_view_policy.rb index 82df100b6735b7827bfc960f1531a13d6d723b8c..8f88c9bd2f64a0028ae6b26bbda85144a471dcd7 100644 --- a/app/policies/work_items/saved_views/saved_view_policy.rb +++ b/app/policies/work_items/saved_views/saved_view_policy.rb @@ -6,6 +6,9 @@ class SavedViewPolicy < BasePolicy # Require users be logged in before they can create, read, update or delete saved views rule { anonymous }.prevent_all + # Require users to be able to read the namespace before they can interact with saved views + rule { ~can_read_namespace }.prevent_all + condition(:can_read_namespace) do can?(:read_namespace, @subject.namespace) end @@ -30,13 +33,14 @@ class SavedViewPolicy < BasePolicy enable :delete_saved_view end - rule { can_read_namespace & is_author }.policy do + rule { is_author }.policy do enable :read_saved_view enable :update_saved_view enable :delete_saved_view + enable :update_saved_view_visibility end - rule { can_read_namespace & ~is_private }.policy do + rule { ~is_private }.policy do enable :read_saved_view end end diff --git a/app/services/work_items/saved_views/create_service.rb b/app/services/work_items/saved_views/create_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..692d4e150c7101394e3c03bc0a50675c76c1baee --- /dev/null +++ b/app/services/work_items/saved_views/create_service.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module WorkItems + module SavedViews + class CreateService < BaseService + attr_reader :current_user, :container, :params + + def initialize(current_user:, container:, params:) + @current_user = current_user + @container = container + @params = params + end + + def execute + unless container&.work_items_saved_views_enabled?(current_user) + return ServiceResponse.error(message: 'Saved views are not enabled for this namespace.') + end + + filter_data = normalize_filters!(params) + + saved_view = ::WorkItems::SavedViews::SavedView.new( + **params, + namespace: container.is_a?(Project) ? container.project_namespace : container, + created_by_id: current_user.id, + filter_data: filter_data, + version: 1 + ) + + if saved_view.save + ServiceResponse.success(payload: { saved_view: saved_view }) + else + ServiceResponse.error(message: saved_view.errors.full_messages) + end + end + + def normalize_filters!(params) + filter_data = FilterNormalizerService.new( + filter_data: params[:filters], container: container, current_user: current_user + ).execute.payload + + params.delete(:filters) + + filter_data + end + end + end +end diff --git a/app/services/work_items/saved_views/filter_base_service.rb b/app/services/work_items/saved_views/filter_base_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..2c06b07d7a4f262ff822da8be5216ab39af90634 --- /dev/null +++ b/app/services/work_items/saved_views/filter_base_service.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module WorkItems + module SavedViews + class FilterBaseService < BaseService + attr_reader :filters, :container, :current_user + + # Filters with values which don't need to be validated on load or save. The values are validated on the GQL layer + # Overridden in EE + def self.static_filters + %i[ + state confidential subscribed issue_types in search my_reaction_emoji closed_before closed_after + assignee_wildcard_id milestone_wildcard_id release_tag_wildcard_id iid due_before due_after created_before + created_after updated_before updated_after release_tag_wildcard_id exclude_projects exclude_group_work_items + include_descendants + ] + end + + def self.static_negated_filters + %i[milestone_wildcard_id my_reaction_emoji issue_types] + end + end + end +end + +WorkItems::SavedViews::FilterBaseService.prepend_mod diff --git a/app/services/work_items/saved_views/filter_normalizer_service.rb b/app/services/work_items/saved_views/filter_normalizer_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..6f7df647d4dc10850f9239d6e4a82c3d62002c0d --- /dev/null +++ b/app/services/work_items/saved_views/filter_normalizer_service.rb @@ -0,0 +1,191 @@ +# frozen_string_literal: true + +module WorkItems + module SavedViews + class FilterNormalizerService < FilterBaseService + include ::API::Concerns::Milestones::GroupProjectParams + + attr_reader :normalized_filters + + def initialize(filter_data:, container:, current_user:) + @filters = filter_data.to_h + @container = container + @current_user = current_user + @normalized_filters = {} + end + + # Overridden in EE + def execute + normalize_static_filters + + normalize_usernames(:assignee_usernames, :assignee_ids) + normalize_author_username + normalize_label_names + normalize_attribute(:milestone_title, :milestone_ids, method: :find_milestone_ids) + normalize_attribute(:release_tag, :release_ids, method: :find_release_ids, + condition: -> { container.is_a?(Project) }) + + normalize_simple_id(:crm_contact_id) + normalize_simple_id(:crm_organization_id) + normalize_full_path + + normalize_hierarchy + + ServiceResponse.success(payload: normalized_filters) + end + + private + + def normalize_static_filters + normalized_filters.merge!(filters.slice(*self.class.static_filters)) + + return unless filters[:not] + + normalized_filters[:not] ||= {} + normalized_filters[:not].merge!(filters[:not].slice(*self.class.static_negated_filters)) + end + + def normalize_usernames(input_key, output_key) + normalize_with_context(input_key, output_key) do |usernames| + User.by_username(usernames).map(&:id) + end + end + + # Normalize attributes with a custom finder method + def normalize_attribute(input_key, output_key, method: nil, condition: nil) + return if condition && !condition.call + + normalize_with_context(input_key, output_key) do |values| + if block_given? + yield values + elsif method + case method + when :find_label_ids then find_label_ids(values) + when :find_milestone_ids then find_milestone_ids(values) + when :find_release_ids then find_release_ids(values) + else + raise ArgumentError, "Unknown normalization method: #{method}" + end + else + raise ArgumentError, "Must provide either method: or a block" + end + end + end + + # Normalize an attribute, and if applicable, it's negation (NOT) and union (OR) + def normalize_with_context(input_key, output_key) + normalized_filters[output_key] = yield(filters[input_key]) if filters[input_key] + + # Negated context + if filters.dig(:not, input_key) + normalized_filters[:not] ||= {} + normalized_filters[:not][output_key] = yield(filters[:not][input_key]) + end + + # Unioned context (OR) + return unless filters.dig(:or, input_key) + + normalized_filters[:or] ||= {} + normalized_filters[:or][output_key] = yield(filters[:or][input_key]) + end + + def normalize_simple_id(key) + return unless filters[key] + + normalized_filters[key] = filters[key] + end + + def find_label_ids(label_names) + root_namespace = container.root_ancestor + descendant_ids = root_namespace.self_and_descendant_ids + + # rubocop: disable CodeReuse/ActiveRecord -- TODO - move to model scope? + group_labels = Label.where(type: 'GroupLabel', group_id: descendant_ids).with_title(label_names) + project_labels = Label.where( + type: 'ProjectLabel', + project_id: Project.in_namespace(descendant_ids).select(:id) + ).with_title(label_names) + # rubocop: enable CodeReuse/ActiveRecord + + Label.from_union([group_labels, project_labels], remove_duplicates: false) + .without_order + .map(&:id) + end + + def find_milestone_ids(titles) + finder_params = { title: titles } + milestones_finder_params = if container.is_a?(Project) + finder_params.merge(project_finder_params(container, { include_ancestors: true })) + else + finder_params.merge(group_finder_params(container, + { include_ancestors: true, include_descendants: true })) + end + + MilestonesFinder.new(params: milestones_finder_params).execute.map(&:id) + end + + def find_release_ids(tags) + ReleasesFinder.new(container, current_user, tag: tags).execute.map(&:id) + end + + def normalize_full_path + return unless filters[:full_path] + + found_routable = Routable.find_by_full_path(filters[:full_path]) + normalized_filters[:routable_id] = found_routable.id if found_routable + end + + def normalize_hierarchy + return unless filters[:hierarchy_filters] + + hierarchy_filters = filters[:hierarchy_filters].to_h + normalized_hierarchy = {} + + if hierarchy_filters[:parent_ids].present? + parent_gids = hierarchy_filters[:parent_ids] + parent_ids = parent_gids.filter_map { |gid| GlobalID.parse(gid)&.model_id } + + normalized_hierarchy[:work_item_parent_ids] = parent_ids if parent_ids.any? + end + + if hierarchy_filters[:parent_wildcard_id].present? + normalized_hierarchy[:parent_wildcard_id] = hierarchy_filters[:parent_wildcard_id] + end + + if hierarchy_filters.key?(:include_descendant_work_items) + normalized_hierarchy[:include_descendant_work_items] = hierarchy_filters[:include_descendant_work_items] + end + + normalized_filters[:hierarchy_filters] = normalized_hierarchy if normalized_hierarchy.any? + end + + def normalize_author_username + # Author is passed differently in the GQL depending on the context (negated / unioned) + normalize_with_context(:author_username, :author_ids) do |username_or_usernames| + usernames = Array.wrap(username_or_usernames) + User.by_username(usernames).map(&:id) + end + + return unless filters.dig(:or, :author_usernames) + + normalized_filters[:or] ||= {} + normalized_filters[:or][:author_ids] = User.by_username(filters[:or][:author_usernames]).map(&:id) + end + + def normalize_label_names + # Label is passed differently in the GQL depending on the context (negated / unioned) + normalize_with_context(:label_name, :label_ids) do |label_name_or_names| + label_names = Array.wrap(label_name_or_names) + find_label_ids(label_names) + end + + return unless filters.dig(:or, :label_names) + + normalized_filters[:or] ||= {} + normalized_filters[:or][:label_ids] = find_label_ids(filters[:or][:label_names]) + end + end + end +end + +WorkItems::SavedViews::FilterNormalizerService.prepend_mod diff --git a/app/services/work_items/saved_views/filter_validator_service.rb b/app/services/work_items/saved_views/filter_validator_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..156ada77c1ef85116b6652a7a35262d0d3334464 --- /dev/null +++ b/app/services/work_items/saved_views/filter_validator_service.rb @@ -0,0 +1,255 @@ +# frozen_string_literal: true + +module WorkItems + module SavedViews + class FilterValidatorService < FilterBaseService + attr_accessor :warnings + attr_reader :sanitized_filters + + def initialize(filter_data:, namespace:, current_user:) + @filters = filter_data.deep_dup.deep_symbolize_keys + @container = namespace + @current_user = current_user + @warnings = [] + @sanitized_filters = {} + end + + def execute + validate_static_filters + + validate_assignee + validate_not_assignee + + validate_author + validate_not_author + validate_or_author + + validate_labels + validate_not_labels + validate_or_labels + + validate_milestone + validate_not_milestone + + validate_hierarchy + + validate_release + validate_negated_release + + validate_crm_organization_id + validate_crm_contact_id + + validate_full_path + + ServiceResponse.success(payload: { filters: sanitized_filters, warnings: @warnings }) + end + + private + + def validate_static_filters + sanitized_filters.merge!(filters.slice(*self.class.static_filters)) + + # Handle static negated filters + return unless filters[:not] + + sanitized_filters[:not] ||= {} + sanitized_filters[:not].merge!(filters[:not].slice(*self.class.static_negated_filters)) + end + + def validate_assignee + validate_id_to_attribute(id_key: :assignee_ids, model: User, attribute: :username, + output_key: :assignee_usernames, warning_label: 'assignee(s)') + end + + def validate_not_assignee + validate_negated_id_to_attribute(id_key: :assignee_ids, model: User, attribute: :username, + output_key: :assignee_usernames, warning_label: 'assignee(s)') + end + + def validate_author + return unless filters[:author_ids] + + found_records = User.id_in(filters[:author_ids]) + found_usernames = found_records.map(&:username) + + missing_count = filters[:author_ids].size - found_records.size + add_warning(:author_username, "#{missing_count} author(s) not found") if missing_count > 0 + + return unless found_usernames.any? + + sanitized_filters[:author_username] = found_usernames.size == 1 ? found_usernames.first : found_usernames + end + + def validate_not_author + validate_negated_id_to_attribute(id_key: :author_ids, model: User, attribute: :username, + output_key: :author_username, warning_label: 'author(s)') + end + + def validate_or_author + return unless filters.dig(:or, :author_ids) + + found_records = User.id_in(filters[:or][:author_ids]) + found_usernames = found_records.map(&:username) + + missing_count = filters[:or][:author_ids].size - found_records.size + add_warning(:or_author_usernames, "#{missing_count} author(s) not found") if missing_count > 0 + + return unless found_usernames.any? + + sanitized_filters[:or] ||= {} + sanitized_filters[:or][:author_usernames] = found_usernames + end + + def validate_labels + validate_id_to_attribute(id_key: :label_ids, model: Label, attribute: :title, output_key: :label_name, + warning_label: 'label(s)', unique: true) + end + + def validate_not_labels + validate_negated_id_to_attribute(id_key: :label_ids, model: Label, attribute: :title, output_key: :label_name, + warning_label: 'label(s)', unique: true) + end + + def validate_or_labels + # Unioned needs custom handling due to different field name (label_names vs label_name) + return unless filters.dig(:or, :label_ids) + + found_records = Label.id_in(filters[:or][:label_ids]) + found_titles = found_records.map(&:title).uniq + + missing_count = filters[:or][:label_ids].size - found_records.size + add_warning(:or_label_names, "#{missing_count} label(s) not found") if missing_count > 0 + + return unless found_titles.any? + + sanitized_filters[:or] ||= {} + sanitized_filters[:or][:label_names] = found_titles + end + + def validate_milestone + validate_id_to_attribute(id_key: :milestone_ids, model: Milestone, attribute: :title, + output_key: :milestone_title, warning_label: 'milestone(s)') + end + + def validate_not_milestone + validate_negated_id_to_attribute(id_key: :milestone_ids, model: Milestone, attribute: :title, + output_key: :milestone_title, warning_label: 'milestone(s)', unique: true) + end + + def validate_group + return unless filters[:group_id] + + group = Group.find_by_id(filters[:group_id]) + + return add_warning(:full_path, "Group not found") unless group + + sanitized_filters[:full_path] = group.full_path + end + + def validate_hierarchy + return unless filters[:hierarchy_filters] + + hierarchy_filters = filters[:hierarchy_filters] + sanitized_hierarchy = {} + + if hierarchy_filters[:work_item_parent_ids].present? + found_parents = WorkItem.id_in(hierarchy_filters[:work_item_parent_ids]) + + missing_count = hierarchy_filters[:work_item_parent_ids].size - found_parents.size + add_warning(:parent_ids, "#{missing_count} parent work item(s) not found") if missing_count > 0 + + if found_parents.any? + sanitized_hierarchy[:parent_ids] = found_parents.map do |parent| + Gitlab::GlobalId.build(parent, id: parent.id).to_s + end + end + end + + if hierarchy_filters[:parent_wildcard_id].present? + sanitized_hierarchy[:parent_wildcard_id] = hierarchy_filters[:parent_wildcard_id] + end + + if hierarchy_filters.key?(:include_descendant_work_items) + sanitized_hierarchy[:include_descendant_work_items] = hierarchy_filters[:include_descendant_work_items] + end + + sanitized_filters[:hierarchy_filters] = sanitized_hierarchy if sanitized_hierarchy.any? + end + + def validate_release + validate_id_to_attribute(id_key: :release_ids, model: Release, attribute: :tag, output_key: :release_tag, + warning_label: 'release(s)') + end + + def validate_negated_release + validate_negated_id_to_attribute(id_key: :release_ids, model: Release, attribute: :tag, + output_key: :release_tag, warning_label: 'release(s)') + end + + def validate_crm_contact_id + return unless filters[:crm_contact_id] + + if CustomerRelations::IssueContact.exists?(contact_id: filters[:crm_contact_id]) # rubocop:disable CodeReuse/ActiveRecord -- TODO: Model scope? + sanitized_filters[:crm_contact_id] = filters[:crm_contact_id] + else + add_warning(:crm_contact_id, "CRM contact not found") + end + end + + def validate_crm_organization_id + return unless filters[:crm_organization_id] + + if CustomerRelations::Contact.exists?(organization_id: filters[:crm_organization_id]) # rubocop:disable CodeReuse/ActiveRecord -- TODO: Model scope? + sanitized_filters[:crm_organization_id] = filters[:crm_organization_id] + else + add_warning(:crm_organization_id, "CRM organization not found") + end + end + + def validate_full_path + return unless filters[:routable_id] + + routable = Group.find_by_id(filters[:routable_id]) || Project.find_by_id(filters[:routable_id]) + + return add_warning(:full_path, "Group / Project not found") unless routable + + sanitized_filters[:full_path] = routable.full_path + end + + def validate_id_to_attribute(id_key:, model:, attribute:, output_key:, warning_label:, unique: false) + return unless filters[id_key] + + found_records = model.id_in(filters[id_key]) + found_attributes = found_records.map(&attribute) + found_attributes = found_attributes.uniq if unique + + missing_count = filters[id_key].size - found_records.size + add_warning(output_key, "#{missing_count} #{warning_label} not found") if missing_count > 0 + + sanitized_filters[output_key] = found_attributes if found_attributes.any? + end + + def validate_negated_id_to_attribute(id_key:, model:, attribute:, output_key:, warning_label:, unique: false) + return unless filters.dig(:not, id_key) + + found_records = model.id_in(filters[:not][id_key]) + found_attributes = found_records.map(&attribute) + found_attributes = found_attributes.uniq if unique + + missing_count = filters[:not][id_key].size - found_records.size + add_warning(:"not_#{output_key}", "#{missing_count} #{warning_label} not found") if missing_count > 0 + + return unless found_attributes.any? + + sanitized_filters[:not] ||= {} + sanitized_filters[:not][output_key] = found_attributes + end + + def add_warning(field, message) + warnings << { field: field, message: message } + end + end + end +end + +WorkItems::SavedViews::FilterValidatorService.prepend_mod diff --git a/app/services/work_items/saved_views/reorder_service.rb b/app/services/work_items/saved_views/reorder_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..68eb26707447ab70fa51462c691fe830dfa3e672 --- /dev/null +++ b/app/services/work_items/saved_views/reorder_service.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module WorkItems + module SavedViews + class ReorderService < BaseService + attr_reader :current_user, :params + + def initialize(current_user:, params:) + @current_user = current_user + @params = params + end + + def execute(saved_view) + namespace = saved_view.namespace + + base_query = ::WorkItems::SavedViews::UserSavedView.in_namespace(namespace).for_user(current_user) + + user_saved_view_to_move = base_query.for_saved_view(saved_view).first + + adjacent_id = params[:move_before_id] || params[:move_after_id] + return ServiceResponse.error(message: "Unable to find subscribed saved view(s)") unless adjacent_id + + adjacent_saved_view = ::WorkItems::SavedViews::SavedView.id_in(adjacent_id).first + return ServiceResponse.error(message: "Unable to find subscribed saved view(s)") unless adjacent_saved_view + + adjacent_user_saved_view = base_query.for_saved_view(adjacent_saved_view).first + + unless user_saved_view_to_move && adjacent_user_saved_view + return ServiceResponse.error(message: "Unable to find subscribed saved view(s)") + end + + if params[:move_before_id] + user_saved_view_to_move.move_between(nil, adjacent_user_saved_view) + else + user_saved_view_to_move.move_between(adjacent_user_saved_view, nil) + end + + user_saved_view_to_move.save! + + ServiceResponse.success(payload: { saved_view: user_saved_view_to_move }) + end + end + end +end diff --git a/app/services/work_items/saved_views/update_service.rb b/app/services/work_items/saved_views/update_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..32a9667bf7019da575ff4ee2531c99b4065dbc29 --- /dev/null +++ b/app/services/work_items/saved_views/update_service.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module WorkItems + module SavedViews + class UpdateService < BaseService + attr_reader :current_user, :saved_view, :params + + def initialize(current_user:, saved_view:, params:) + @current_user = current_user + @saved_view = saved_view + @params = params + end + + def execute + return ServiceResponse.error(message: 'Saved views are not enabled for this namespace.') unless feature_enabled? + return visibility_authorization_error if updating_visibility? && !can_update_visibility? + + normalize_filters! if params[:filters] + + should_unsubscribe = changing_to_private? + + SavedView.transaction do + if saved_view.update(params) + saved_view.unsubscribe_other_users!(user: current_user) if should_unsubscribe + ServiceResponse.success(payload: { saved_view: saved_view }) + else + ServiceResponse.error(message: saved_view.errors.full_messages) + end + end + end + + private + + def feature_enabled? + parent.work_items_saved_views_enabled?(current_user) + end + + def updating_visibility? + params.key?(:private) + end + + def can_update_visibility? + Ability.allowed?(current_user, :update_saved_view_visibility, saved_view) + end + + def visibility_authorization_error + ServiceResponse.error(message: 'Only the author can change visibility settings') + end + + def normalize_filters! + result = FilterNormalizerService.new( + filter_data: params[:filters], + current_user: current_user, + container: parent + ).execute + + params[:filter_data] = result.payload + params.delete(:filters) + end + + def changing_to_private? + params[:private] == true && !saved_view.private? + end + + def parent + @parent ||= saved_view.namespace.owner_entity + end + end + end +end diff --git a/app/validators/json_schemas/saved_view_display_settings.json b/app/validators/json_schemas/saved_view_display_settings.json new file mode 100644 index 0000000000000000000000000000000000000000..52aeb19d46fc423d4e4c352f1a49cd1efcbb207c --- /dev/null +++ b/app/validators/json_schemas/saved_view_display_settings.json @@ -0,0 +1,28 @@ +{ + "description": "Display settings data for a saved view", + "type": "object", + "properties": { + "hiddenMetadataKeys": { + "type": "array", + "items": { + "type": "string", + "enum": [ + "assignee", + "blocked", + "blocking", + "dates", + "health", + "labels", + "milestone", + "popularity", + "weight", + "comments", + "iteration", + "status" + ] + }, + "uniqueItems": true, + "additionalProperties": false + } + } +} diff --git a/app/validators/json_schemas/saved_view_filters.json b/app/validators/json_schemas/saved_view_filters.json new file mode 100644 index 0000000000000000000000000000000000000000..e0da74cccc1d84d842c218e249928b2c22640d3f --- /dev/null +++ b/app/validators/json_schemas/saved_view_filters.json @@ -0,0 +1,313 @@ +{ + "description": "Filter data for a saved view", + "type": "object", + "properties": { + "assignee_wildcard_id": { + "type": "string" + }, + "iteration_wildcard_id": { + "type": "string" + }, + "release_tag_wildcard_id": { + "type": "string" + }, + "assignee_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "author_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "iid": { + "type": "string" + }, + "confidential": { + "type": "boolean" + }, + "routable_id": { + "type": "integer" + }, + "health_status_filter": { + "type": "string" + }, + "milestone_wildcard_id": { + "type": "string" + }, + "iteration_id": { + "type": "array", + "items": { + "type": "string" + } + }, + "label_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "milestone_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "my_reaction_emoji": { + "type": "string" + }, + "parent": { + "type": "string" + }, + "release_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "in": { + "type": "array", + "items": { + "type": "string" + } + }, + "search": { + "type": "string" + }, + "state": { + "type": "string" + }, + "status": { + "type": "string" + }, + "subscribed": { + "type": "string" + }, + "custom_field": { + "type": "array", + "items": { + "type": "object", + "properties": { + "custom_field_id": { + "type": "string" + }, + "selected_option_ids": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + }, + "issue_types": { + "type": "array", + "items": { + "type": "string" + } + }, + "weight": { + "type": "string" + }, + "group": { + "type": "string" + }, + "crm_organization_id": { + "type": "string" + }, + "crm_contact_id": { + "type": "string" + }, + "created_before": { + "type": "string" + }, + "created_after": { + "type": "string" + }, + "updated_before": { + "type": "string" + }, + "updated_after": { + "type": "string" + }, + "closed_before": { + "type": "string" + }, + "closed_after": { + "type": "string" + }, + "due_before": { + "type": "string" + }, + "due_after": { + "type": "string" + }, + "exclude_projects": { + "type": "boolean" + }, + "exclude_group_work_items": { + "type": "boolean" + }, + "include_descendants": { + "type": "boolean" + }, + "hierarchy_filters": { + "type": "object", + "properties": { + "parent_ids": { + "type": "array", + "items": { + "type": "string" + } + }, + "parent_wildcard_id": { + "type": "string" + }, + "include_descendant_work_items": { + "type": "boolean" + } + }, + "additionalProperties": false + }, + "iteration_cadence_ids": { + "type": "array", + "items": { + "type": "string" + } + }, + "not": { + "type": "object", + "properties": { + "assignee_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "iteration_wildcard_id": { + "type": "string" + }, + "author_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "label_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "milestone_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "milestone_wildcard_id": { + "type": "string" + }, + "release_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "iteration_cadence_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "my_reaction_emoji": { + "type": "string" + }, + "issue_types": { + "type": "array", + "items": { + "type": "string" + } + }, + "health_status_filter": { + "type": "array", + "items": { + "type": "string" + } + }, + "weight": { + "type": "string" + }, + "custom_field": { + "type": "array", + "items": { + "type": "object", + "properties": { + "custom_field_id": { + "type": "string" + }, + "selected_option_ids": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + }, + "iteration_id": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "additionalProperties": false + }, + "or": { + "type": "object", + "properties": { + "assignee_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "author_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "label_ids": { + "type": "array", + "items": { + "type": "integer" + } + }, + "custom_field": { + "type": "array", + "items": { + "type": "object", + "properties": { + "custom_field_id": { + "type": "string" + }, + "selected_option_ids": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } + }, + "additionalProperties": false + } + }, + "additionalProperties": false +} diff --git a/config/authz/permissions/saved_view_visibility/_metadata.yml b/config/authz/permissions/saved_view_visibility/_metadata.yml new file mode 100644 index 0000000000000000000000000000000000000000..d7021291d2cd1f1700395ccef2fa16fde0aa21ba --- /dev/null +++ b/config/authz/permissions/saved_view_visibility/_metadata.yml @@ -0,0 +1,2 @@ +--- +feature_category: portfolio_management diff --git a/config/authz/permissions/saved_view_visibility/update.yml b/config/authz/permissions/saved_view_visibility/update.yml new file mode 100644 index 0000000000000000000000000000000000000000..e0632ced7dc8a6dc4822206bdbb95f973d2a844f --- /dev/null +++ b/config/authz/permissions/saved_view_visibility/update.yml @@ -0,0 +1,4 @@ +--- +name: update_saved_view_visibility +description: Grants the ability to update the visibility of a saved view +feature_category: portfolio_management diff --git a/config/routes/group.rb b/config/routes/group.rb index 91ae9330983ee00f17a1aece172c3180380be6c8..e04b3fcd0eeaa5160ffff334765ae9ef6eac7821 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -39,11 +39,7 @@ get :work_items, to: 'work_items#rss', constraints: ->(req) { req.format == :atom } get :work_items, to: 'work_items#calendar', constraints: ->(req) { req.format == :ics } - resources :saved_views, only: [], path: 'work_items/saved_views' do - member do - get :subscribe - end - end + resources :saved_views, only: [:show], path: 'work_items/saved_views' namespace :settings do resource :ci_cd, only: [:show, :update], controller: 'ci_cd' do diff --git a/config/routes/project.rb b/config/routes/project.rb index bf738c26f2b9ab2e17bd9c4f08cac16bd632f111..ba3da8a483f474a5473860c09693430a076fba6d 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -361,11 +361,7 @@ get :work_items, to: 'work_items#calendar', constraints: ->(req) { req.format == :ics } get :work_items, to: 'work_items#rss', constraints: ->(req) { req.format == :atom } - resources :saved_views, only: [], path: 'work_items/saved_views' do - member do - get :subscribe - end - end + resources :saved_views, only: [:show], path: 'work_items/saved_views' resources :work_items, only: [:show, :index, :edit], param: :iid do collection do diff --git a/ee/app/graphql/ee/types/work_items/saved_views/negated_saved_view_filter_input_type.rb b/ee/app/graphql/ee/types/work_items/saved_views/negated_saved_view_filter_input_type.rb new file mode 100644 index 0000000000000000000000000000000000000000..25f4296c67c0a8f12e072d747a78385798dd5a29 --- /dev/null +++ b/ee/app/graphql/ee/types/work_items/saved_views/negated_saved_view_filter_input_type.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module EE + module Types + module WorkItems + module SavedViews + module NegatedSavedViewFilterInputType + extend ActiveSupport::Concern + + prepended do + argument :health_status_filter, + [::Types::HealthStatusEnum], + required: false, + description: 'Filter values for the not health status filter.' + argument :weight, + GraphQL::Types::String, + required: false, + description: 'Filter values for the not weight filter.' + argument :iteration_id, + [::GraphQL::Types::ID], + required: false, + validates: { length: { maximum: ::WorkItems::SharedFilterArguments::MAX_FIELD_LIMIT } }, + description: "Filter values for the not iteration id filter. " \ + "(maximum is #{::WorkItems::SharedFilterArguments::MAX_FIELD_LIMIT} IDs)." + argument :iteration_wildcard_id, + ::Types::IterationWildcardIdEnum, + required: false, + description: 'Filter value for the not iteration wildcard id filter.' + argument :custom_field, + [::Types::WorkItems::Widgets::CustomFieldFilterInputType], + required: false, + description: 'Filter value for the not custom field filter.' + + validates mutually_exclusive: [:iteration_id, :iteration_wildcard_id] + end + end + end + end + end +end diff --git a/ee/app/graphql/ee/types/work_items/saved_views/saved_view_filter_input_type.rb b/ee/app/graphql/ee/types/work_items/saved_views/saved_view_filter_input_type.rb new file mode 100644 index 0000000000000000000000000000000000000000..e981f67f2888ce096cc1b4e45172c5d258ee1bba --- /dev/null +++ b/ee/app/graphql/ee/types/work_items/saved_views/saved_view_filter_input_type.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +module EE + module Types + module WorkItems + module SavedViews + module SavedViewFilterInputType + extend ActiveSupport::Concern + + prepended do + argument :custom_field, + [::Types::WorkItems::Widgets::CustomFieldFilterInputType], + required: false, + validates: { length: { maximum: ::WorkItems::SharedFilterArguments::MAX_FIELD_LIMIT } }, + description: "Filter value for custom fields filter. " \ + "(maximum is #{::WorkItems::SharedFilterArguments::MAX_FIELD_LIMIT} fields)." + argument :health_status_filter, + ::Types::HealthStatusFilterEnum, + required: false, + description: 'Filter value for health status filter.' + argument :iteration_id, + [::GraphQL::Types::ID], + required: false, + description: 'Filter value for iteration id filter.', + validates: { length: { maximum: ::WorkItems::SharedFilterArguments::MAX_FIELD_LIMIT } } + argument :iteration_wildcard_id, + ::Types::IterationWildcardIdEnum, + required: false, + description: 'Filter value for iteration wildcard filter.' + argument :status, + ::Types::WorkItems::Widgets::StatusFilterInputType, + required: false, + description: 'Filter value for status filter.' + argument :weight, + GraphQL::Types::String, + required: false, + description: 'Filter value for weight filter.' + argument :weight_wildcard_id, + ::Types::WeightWildcardIdEnum, + required: false, + description: 'Filter value for weight wildcard filter.' + argument :iteration_cadence_id, + [::Types::GlobalIDType[::Iterations::Cadence]], + required: false, + validates: { length: { maximum: ::WorkItems::SharedFilterArguments::MAX_FIELD_LIMIT } }, + description: "Filter value for iteration cadence id filter. " \ + "(maximum is #{::WorkItems::SharedFilterArguments::MAX_FIELD_LIMIT} IDs).", + prepare: ->(iteration_cadence_ids, _ctx) { + return unless iteration_cadence_ids.present? + + iteration_cadence_ids.map do |arg| + GitlabSchema.parse_gid(arg, expected_type: ::Iterations::Cadence).model_id + end + } + + validates mutually_exclusive: [:weight, :weight_wildcard_id] + validates mutually_exclusive: [:iteration_id, :iteration_wildcard_id] + end + end + end + end + end +end diff --git a/ee/app/models/ee/work_items/saved_views/user_saved_view.rb b/ee/app/models/ee/work_items/saved_views/user_saved_view.rb index 733b3c5b170230644f01fccdc61d4b63c7e6b917..90997c1d3d2dc656a7b0fd07b2eb1d9141197b98 100644 --- a/ee/app/models/ee/work_items/saved_views/user_saved_view.rb +++ b/ee/app/models/ee/work_items/saved_views/user_saved_view.rb @@ -5,6 +5,7 @@ module WorkItems module SavedViews module UserSavedView extend ActiveSupport::Concern + class_methods do extend ::Gitlab::Utils::Override diff --git a/ee/app/services/ee/work_items/saved_views/filter_base_service.rb b/ee/app/services/ee/work_items/saved_views/filter_base_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..71f0663892fee3b349bae2c0d4f6b24ee09e94d9 --- /dev/null +++ b/ee/app/services/ee/work_items/saved_views/filter_base_service.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module EE + module WorkItems + module SavedViews + module FilterBaseService + extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override + + class_methods do + extend ::Gitlab::Utils::Override + + override :static_filters + def static_filters + super + %i[health_status_filter iteration_wildcard_id weight] + end + + override :static_negated_filters + def static_negated_filters + super + %i[health_status_filter weight iteration_wildcard_id] + end + end + end + end + end +end diff --git a/ee/app/services/ee/work_items/saved_views/filter_normalizer_service.rb b/ee/app/services/ee/work_items/saved_views/filter_normalizer_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..6792c780a9ba8f71c6809fc19d8c84a86e554ad3 --- /dev/null +++ b/ee/app/services/ee/work_items/saved_views/filter_normalizer_service.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +module EE + module WorkItems + module SavedViews + module FilterNormalizerService + extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override + + override :execute + def execute + super + + # Custom fields are already passed as IDs, check if they exist when reading the saved view + normalize_attribute(:custom_field, :custom_field) { |values| normalize_custom_field_entries(values) } + normalize_attribute(:iteration_id, :iteration_id) { |value| value } + + normalize_iteration_cadence_id + normalize_status + + ServiceResponse.success(payload: normalized_filters) + end + + private + + def normalize_custom_field_entries(custom_fields) + custom_fields.filter_map do |cf| + normalize_single_custom_field(cf) + end + end + + def normalize_single_custom_field(cf) + # Normalize custom field + custom_field_id = if cf[:custom_field_id] + cf[:custom_field_id] + elsif cf[:custom_field_name] + find_custom_field_id_by_name(cf[:custom_field_name]) + end + + return unless custom_field_id + + # Normalize selected options + selected_option_ids = if cf[:selected_option_ids] + cf[:selected_option_ids] + elsif cf[:selected_option_values] + find_option_ids_by_values(custom_field_id, cf[:selected_option_values]) + end + + return unless selected_option_ids&.any? + + { + custom_field_id: custom_field_id.to_s, + selected_option_ids: selected_option_ids.map(&:to_s) + } + end + + def find_custom_field_id_by_name(name) + root_namespace = container.root_ancestor + descendant_ids = root_namespace.self_and_descendant_ids + + descendant_ids.each do |namespace_id| + custom_field = ::Issuables::CustomField + .of_namespace(namespace_id) + .find_by_case_insensitive_name(name) + + return custom_field.id if custom_field + end + + nil + end + + def find_option_ids_by_values(custom_field_id, values) + ::Issuables::CustomFieldSelectOption + .of_field(custom_field_id) + .with_case_insensitive_values(values) + .map(&:id) + end + + def normalize_iteration_cadence_id + # Note: input is iteration_cadence_id, output is iteration_cadence_ids (plural) + return unless filters[:iteration_cadence_id] + + normalized_filters[:iteration_cadence_ids] = filters[:iteration_cadence_id] + end + + def normalize_status + return unless filters[:status] + + status_filter = filters[:status] + + # Status can be provided as either an ID or by name. We store the value as an ID regardless, but also the + # format it was provided in + if status_filter.is_a?(::WorkItems::Statuses::Status) + normalized_filters[:status] = { + id: status_filter.id, + input_format: :id + } + elsif status_filter.is_a?(Hash) && status_filter[:name] + status_id = find_status_id_by_name(status_filter[:name]) + if status_id + normalized_filters[:status] = { + id: status_id, + input_format: :name + } + end + end + end + + def find_status_id_by_name(name) + root_namespace = container.root_ancestor + + status = ::WorkItems::Statuses::Finder.new( + root_namespace, { 'name' => name }, + current_user: current_user + ).execute.first + + status&.id + end + end + end + end +end diff --git a/ee/app/services/ee/work_items/saved_views/filter_validator_service.rb b/ee/app/services/ee/work_items/saved_views/filter_validator_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..88d4d2a5e578b15513d9196bfc65780ead510704 --- /dev/null +++ b/ee/app/services/ee/work_items/saved_views/filter_validator_service.rb @@ -0,0 +1,141 @@ +# frozen_string_literal: true + +module EE + module WorkItems + module SavedViews + module FilterValidatorService + extend ActiveSupport::Concern + extend ::Gitlab::Utils::Override + + override :execute + def execute + super + + validate_custom_fields + validate_negated_custom_fields + + validate_iteration + validate_negated_iteration + validate_iteration_cadence_id + + validate_status + + ServiceResponse.success(payload: { filters: sanitized_filters, warnings: warnings }) + end + + private + + def validate_custom_fields + validate_custom_field_filter(:custom_field, :custom_field) + end + + def validate_negated_custom_fields + validate_custom_field_filter([:not, :custom_field], :not_custom_field) + end + + def validate_custom_field_filter(filter_path, warning_key) + filter_value = filter_path.is_a?(Array) ? filters.dig(*filter_path) : filters[filter_path] + return unless filter_value + + output_location = initialize_custom_field_output(filter_path) + + filter_value.each do |cf| + custom_field_id = cf[:custom_field_id].to_i + selected_option_ids = cf[:selected_option_ids].map(&:to_i) + + valid_options = validate_custom_field_options(custom_field_id, selected_option_ids, warning_key) + next if valid_options.empty? + + output_location[:custom_field] << { + custom_field_id: custom_field_id.to_s, + selected_option_ids: valid_options.map(&:to_s) + } + end + end + + def initialize_custom_field_output(filter_path) + if filter_path.is_a?(Array) && filter_path.first == :not + sanitized_filters[:not] ||= {} + output_location = sanitized_filters[:not] + else + output_location = sanitized_filters + end + + output_location[:custom_field] = [] + output_location + end + + def validate_custom_field_options(custom_field_id, selected_option_ids, warning_key) + valid_option_ids = find_valid_option_ids(custom_field_id, selected_option_ids) + + if valid_option_ids.empty? + add_warning(warning_key, "Custom field #{custom_field_id} not found") + return [] + end + + missing_count = selected_option_ids.size - valid_option_ids.size + if missing_count > 0 + add_warning(warning_key, "#{missing_count} option(s) not found for custom field #{custom_field_id}") + end + + valid_option_ids + end + + def find_valid_option_ids(custom_field_id, selected_option_ids) + ::Issuables::CustomFieldSelectOption + .of_field(custom_field_id) + .id_in(selected_option_ids) + .map(&:id) + end + + def validate_iteration + return unless filters[:iteration_id] + + found_ids = ::Iteration.id_in(filters[:iteration_id]).map { |i| i.id.to_s } + + missing_count = filters[:iteration_id].size - found_ids.size + add_warning(:iteration_id, "#{missing_count} iteration(s) not found") if missing_count > 0 + + sanitized_filters[:iteration_id] = found_ids if found_ids.any? + end + + def validate_negated_iteration + return unless filters.dig(:not, :iteration_id) + + found_ids = ::Iteration.id_in(filters[:not][:iteration_id]).map { |i| i.id.to_s } + + missing_count = filters[:iteration_id].size - found_ids.size + add_warning(:not_iteration_id, "#{missing_count} iteration(s) not found") if missing_count > 0 + + sanitized_filters[:not] ||= {} + sanitized_filters[:not][:iteration_id] = found_ids if found_ids.any? + end + + def validate_status + return unless filters[:status] + + finder = ::WorkItems::Statuses::Finder.new( + container.root_ancestor, + params: { 'name' => filters[:status] }, + current_user: current_user + ) + + return add_warning(:status, "Status not found") if finder.execute.none? + + sanitized_filters[:status] = filters[:status] + end + + def validate_iteration_cadence_id + return unless filters[:iteration_cadence_ids] + + found_ids = ::Iterations::Cadence.id_in(filters[:iteration_cadence_ids]).map { |i| i.to_gid.to_s } + + missing_count = filters[:iteration_cadence_ids].size - found_ids.size + add_warning(:iteration_cadence_id, "#{missing_count} iteration cadence(s) not found") if missing_count > 0 + + sanitized_filters[:iteration_cadence_id] = found_ids if found_ids.any? + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 75994af249ee298a4fc212ffbe63bfe8291ce0a3..dd196f63fa3bf2a42cb3cbfec08551c6a84424a2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -71435,6 +71435,9 @@ msgstr "" msgid "Ultimate" msgstr "" +msgid "Unable to add view. You have reached the maximum number of saved views." +msgstr "" + msgid "Unable to apply suggestions to a deleted line." msgstr "" diff --git a/spec/factories/work_items/saved_views/saved_views.rb b/spec/factories/work_items/saved_views/saved_views.rb index 4e53612a8cec8bbcbeb24be188e94b44cf063750..d37f048d918018dae1d3ab00f3e436c351965c11 100644 --- a/spec/factories/work_items/saved_views/saved_views.rb +++ b/spec/factories/work_items/saved_views/saved_views.rb @@ -8,5 +8,7 @@ author { association(:user) } private { false } version { 1 } + filter_data { { state: 'opened', confidential: true } } + display_settings { {} } end end diff --git a/spec/factories/work_items/saved_views/user_saved_views.rb b/spec/factories/work_items/saved_views/user_saved_views.rb index 84670bd1f62f16f9b0d0d324f3e421af43004ae4..9ed3940bcd04d3fdcee2014bdd11e076fdcd64a6 100644 --- a/spec/factories/work_items/saved_views/user_saved_views.rb +++ b/spec/factories/work_items/saved_views/user_saved_views.rb @@ -2,8 +2,11 @@ FactoryBot.define do factory :user_saved_view, class: 'WorkItems::SavedViews::UserSavedView' do - namespace { association(:group) } user { association(:user) } saved_view { association(:saved_view) } + + after(:build) do |user_saved_view| + user_saved_view.namespace = user_saved_view.saved_view.namespace + end end end diff --git a/spec/graphql/mutations/work_items/saved_views/create_spec.rb b/spec/graphql/mutations/work_items/saved_views/create_spec.rb index e765892ad470cd0caaae3735b424828d9bea61f5..5464e302d0bfd8d4ac4f9dcb4b5333370452a477 100644 --- a/spec/graphql/mutations/work_items/saved_views/create_spec.rb +++ b/spec/graphql/mutations/work_items/saved_views/create_spec.rb @@ -13,7 +13,7 @@ name: 'New Saved View', filters: {}, display_settings: {}, - sort: 'CREATED_ASC' } + sort: :created_asc } end let_it_be(:project) { create(:project) } diff --git a/spec/graphql/mutations/work_items/saved_views/delete_spec.rb b/spec/graphql/mutations/work_items/saved_views/delete_spec.rb index ee7470ce9cc48b8f070fbb8474f7d25593ecefb6..369bdf47f90dd70322d99e238ae6951ccbeebb97 100644 --- a/spec/graphql/mutations/work_items/saved_views/delete_spec.rb +++ b/spec/graphql/mutations/work_items/saved_views/delete_spec.rb @@ -6,27 +6,89 @@ include GraphqlHelpers let_it_be(:project) { create(:project) } - let_it_be(:current_user) { create(:user) } - let_it_be(:saved_view) { create(:saved_view, namespace: project.project_namespace) } - let(:arguments) { { id: saved_view.to_gid } } + let_it_be(:current_user) { create(:user, planner_of: [project]) } + let_it_be_with_reload(:saved_view) { create(:saved_view, namespace: project.project_namespace) } - subject(:mutation) { described_class.new(object: nil, context: query_context, field: nil) } + let(:current_ctx) { { current_user: current_user } } - context 'when the user is not logged in' do - let(:current_user) { nil } + def resolve_mutation(**args) + resolve(described_class, args: args, ctx: current_ctx) + end + + describe '#resolve' do + context 'when the user is not logged in' do + let(:current_ctx) { { current_user: nil } } - it 'raises an appropriate error' do - expect { mutation.resolve(**arguments) }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + it 'raises an appropriate error' do + expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do + resolve_mutation(id: saved_view.to_global_id) + end + end end - end - context 'when the user is logged in and has planner permissions in the project' do - before_all do - project.add_planner(current_user) + context 'when the user does not have permission' do + let_it_be(:guest_user) { create(:user) } + let_it_be_with_reload(:saved_view) { create(:saved_view, namespace: project.project_namespace, private: false) } + let(:current_ctx) { { current_user: guest_user } } + + before_all do + project.add_guest(guest_user) + end + + it 'raises an appropriate error' do + expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do + resolve_mutation(id: saved_view.to_global_id) + end + end + end + + context 'when saved views are not enabled for the namespace' do + before do + stub_feature_flags(work_items_saved_views: false) + end + + it 'returns an error' do + result = resolve_mutation(id: saved_view.to_global_id) + + expect(result[:saved_view]).to be_nil + expect(result[:errors]).to eq(['Saved views are not enabled for this namespace.']) + end end - it 'does not raise an error' do - expect { mutation.resolve(**arguments) }.not_to raise_error + context 'when the user has permission' do + it 'successfully deletes the saved view' do + result = resolve_mutation(id: saved_view.to_global_id) + + expect(result[:saved_view]).to eq(saved_view) + expect(result[:errors]).to be_empty + end + + it 'deletes the saved view record' do + expect { resolve_mutation(id: saved_view.to_global_id) } + .to change { WorkItems::SavedViews::SavedView.count }.by(-1) + end + + it 'removes the saved view from the database' do + saved_view_id = saved_view.id + resolve_mutation(id: saved_view.to_global_id) + + expect(WorkItems::SavedViews::SavedView.find_by(id: saved_view_id)).to be_nil + end + + context 'when users are subscribed to the saved view' do + let_it_be(:user1) { create(:user) } + let_it_be(:user2) { create(:user) } + + before_all do + create(:user_saved_view, user: user1, saved_view: saved_view) + create(:user_saved_view, user: user2, saved_view: saved_view) + end + + it 'also deletes the associated user saved views' do + expect { resolve_mutation(id: saved_view.to_global_id) } + .to change { WorkItems::SavedViews::UserSavedView.count }.by(-2) + end + end end end end diff --git a/spec/graphql/mutations/work_items/saved_views/reorder_spec.rb b/spec/graphql/mutations/work_items/saved_views/reorder_spec.rb index 94945298ed4a46c7a6a39fcc5f1d0619fa08877a..11f7ef5d9b6f85fbd69133da764721ae8aa82e1b 100644 --- a/spec/graphql/mutations/work_items/saved_views/reorder_spec.rb +++ b/spec/graphql/mutations/work_items/saved_views/reorder_spec.rb @@ -7,49 +7,157 @@ let_it_be(:current_user) { create(:user) } let_it_be(:project) { create(:project) } - let_it_be(:saved_views) { create_list(:saved_view, 3, namespace: project.project_namespace) } + let_it_be(:namespace) { project.project_namespace } - let(:arguments) do - { id: saved_views[0].to_gid, move_before_id: saved_views[1].to_gid, move_after_id: saved_views[2].to_gid } + let_it_be(:sv_1) { create(:saved_view, namespace: namespace) } + let_it_be(:sv_2) { create(:saved_view, namespace: namespace) } + let_it_be(:sv_3) { create(:saved_view, namespace: namespace) } + let_it_be(:usv_1) { create(:user_saved_view, user: current_user, saved_view: sv_1, relative_position: 100) } + let_it_be(:usv_2) { create(:user_saved_view, user: current_user, saved_view: sv_2, relative_position: 200) } + let_it_be(:usv_3) { create(:user_saved_view, user: current_user, saved_view: sv_3, relative_position: 300) } + + let(:current_ctx) { { current_user: current_user } } + + before_all do + project.add_planner(current_user) end - subject(:mutation) { described_class.new(object: nil, context: query_context, field: nil) } + def resolve_mutation(**args) + resolve(described_class, args: args, ctx: current_ctx) + end - context 'when the user is not logged in' do - let(:current_user) { nil } + describe '#resolve' do + context 'when the user is not logged in' do + let(:current_ctx) { { current_user: nil } } - it 'raises an appropriate error' do - expect { mutation.resolve(**arguments) }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + it 'raises an appropriate error' do + expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do + resolve_mutation(id: sv_1.to_global_id, move_before_id: sv_2.to_global_id) + end + end end - end - context 'when the user is logged in and has planner permissions in the project' do - before_all do - project.add_planner(current_user) + context 'when the user does not have permission' do + let_it_be(:guest_user) { create(:user) } + let(:current_ctx) { { current_user: guest_user } } + + before_all do + project.add_guest(guest_user) + end + + it 'returns an error because the user is not subscribed to the saved views' do + result = resolve_mutation(id: sv_1.to_global_id, move_before_id: sv_2.to_global_id) + + expect(result[:saved_view]).to be_nil + expect(result[:errors]).to eq(['Unable to find subscribed saved view(s)']) + end + end + + context 'when saved views are not enabled for the namespace' do + before do + stub_feature_flags(work_items_saved_views: false) + end + + it 'returns an error' do + result = resolve_mutation(id: sv_1.to_global_id, move_before_id: sv_2.to_global_id) + + expect(result[:saved_view]).to be_nil + expect(result[:errors]).to eq(['Saved views are not enabled for this namespace.']) + end end - it 'does not raise an error' do - expect { mutation.resolve(**arguments) }.not_to raise_error + context 'when moving a saved view before another' do + it 'successfully reorders the saved view' do + result = resolve_mutation(id: sv_3.to_global_id, move_before_id: sv_1.to_global_id) + + expect(result[:saved_view]).to eq(sv_3) + expect(result[:errors]).to be_empty + end + + it 'updates the relative position' do + expect { resolve_mutation(id: sv_3.to_global_id, move_before_id: sv_1.to_global_id) } + .to change { usv_3.reload.relative_position } + end + + it 'maintains the correct order' do + resolve_mutation(id: sv_3.to_global_id, move_before_id: sv_1.to_global_id) + + user_saved_views = WorkItems::SavedViews::UserSavedView + .in_namespace(namespace) + .for_user(current_user) + .order(:relative_position) + + expect(user_saved_views.map(&:saved_view_id)).to eq([sv_3.id, sv_1.id, sv_2.id]) + end + end + + context 'when moving a saved view after another' do + it 'successfully reorders the saved view' do + result = resolve_mutation(id: sv_1.to_global_id, move_after_id: sv_3.to_global_id) + + expect(result[:saved_view]).to eq(sv_1) + expect(result[:errors]).to be_empty + end + + it 'maintains the correct order' do + resolve_mutation(id: sv_1.to_global_id, move_after_id: sv_3.to_global_id) + + user_saved_views = WorkItems::SavedViews::UserSavedView + .in_namespace(namespace) + .for_user(current_user) + .order(:relative_position) + + expect(user_saved_views.map(&:saved_view_id)).to eq([sv_2.id, sv_3.id, sv_1.id]) + end + end + + context 'when both move_before_id and move_after_id are provided' do + it 'raises a validation error' do + expect_graphql_error_to_be_created(GraphQL::Schema::Validator::ValidationFailedError, + 'Only one of [moveBeforeId, moveAfterId] arguments is allowed at the same time.') do + resolve_mutation(id: sv_1.to_global_id, move_before_id: sv_2.to_global_id, move_after_id: sv_3.to_global_id) + end + end + end + + context 'when neither move_before_id nor move_after_id is provided' do + it 'returns an error from the service' do + result = resolve_mutation(id: sv_1.to_global_id) + + expect(result[:saved_view]).to be_nil + expect(result[:errors]).to eq(['Unable to find subscribed saved view(s)']) + end + end + + context 'when the user is not subscribed to the saved view' do + let_it_be(:unsubscribed_sv) { create(:saved_view, namespace: namespace) } + + it 'returns an error from the service' do + result = resolve_mutation(id: unsubscribed_sv.to_global_id, move_before_id: sv_1.to_global_id) + + expect(result[:saved_view]).to be_nil + expect(result[:errors]).to eq(['Unable to find subscribed saved view(s)']) + end end end describe 'prepare lambdas' do it 'prepares move_before_id to extract model_id from GlobalID' do move_before_arg = described_class.arguments['moveBeforeId'] - gid = saved_views[1].to_gid + gid = sv_1.to_global_id prepared_value = move_before_arg.prepare.call(gid, nil) - expect(prepared_value).to eq(saved_views[1].id.to_s) + expect(prepared_value).to eq(sv_1.id) end it 'prepares move_after_id to extract model_id from GlobalID' do move_after_arg = described_class.arguments['moveAfterId'] - gid = saved_views[2].to_gid + gid = sv_2.to_global_id prepared_value = move_after_arg.prepare.call(gid, nil) - expect(prepared_value).to eq(saved_views[2].id.to_s) + expect(prepared_value).to eq(sv_2.id) end end end diff --git a/spec/graphql/mutations/work_items/saved_views/subscribe_spec.rb b/spec/graphql/mutations/work_items/saved_views/subscribe_spec.rb index 5308d38e9c0b7336b11f7a1180df8c3e8b37f3c4..9df400d5c3d75a355e71f4155d179f614deaf7d2 100644 --- a/spec/graphql/mutations/work_items/saved_views/subscribe_spec.rb +++ b/spec/graphql/mutations/work_items/saved_views/subscribe_spec.rb @@ -6,27 +6,129 @@ include GraphqlHelpers let_it_be(:project) { create(:project) } - let_it_be(:current_user) { create(:user) } + let_it_be(:current_user) { create(:user, planner_of: [project]) } let_it_be(:saved_view) { create(:saved_view, namespace: project.project_namespace) } - let(:arguments) { { id: saved_view.to_gid } } - subject(:mutation) { described_class.new(object: nil, context: query_context, field: nil) } + let(:current_ctx) { { current_user: current_user } } - context 'when the user is not logged in' do - let(:current_user) { nil } + def resolve_mutation(**args) + resolve(described_class, args: args, ctx: current_ctx) + end + + describe '#resolve' do + context 'when the user is not logged in' do + let(:current_ctx) { { current_user: nil } } - it 'raises an appropriate error' do - expect { mutation.resolve(**arguments) }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + it 'raises an appropriate error' do + expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do + resolve_mutation(id: saved_view.to_global_id) + end + end end - end - context 'when the user is logged in and has planner permissions in the project' do - before_all do - project.add_planner(current_user) + context 'when the user is a guest' do + let_it_be(:guest_user) { create(:user) } + let(:current_ctx) { { current_user: guest_user } } + + before_all do + project.add_guest(guest_user) + end + + it 'allows subscription to the saved view' do + result = resolve_mutation(id: saved_view.to_global_id) + + expect(result[:saved_view]).to eq(saved_view) + expect(result[:errors]).to be_empty + end + + it 'creates a user saved view record' do + expect { resolve_mutation(id: saved_view.to_global_id) } + .to change { WorkItems::SavedViews::UserSavedView.count }.by(1) + end end - it 'does not raise an error' do - expect { mutation.resolve(**arguments) }.not_to raise_error + context 'when saved views are not enabled for the namespace' do + before do + stub_feature_flags(work_items_saved_views: false) + end + + it 'returns an error' do + result = resolve_mutation(id: saved_view.to_global_id) + + expect(result[:saved_view]).to be_nil + expect(result[:errors]).to eq(['Saved views are not enabled for this namespace.']) + end + end + + context 'when the user has permission' do + it 'successfully subscribes to the saved view' do + result = resolve_mutation(id: saved_view.to_global_id) + + expect(result[:saved_view]).to eq(saved_view) + expect(result[:errors]).to be_empty + end + + it 'creates a user saved view record' do + expect { resolve_mutation(id: saved_view.to_global_id) } + .to change { WorkItems::SavedViews::UserSavedView.count }.by(1) + end + + it 'sets the correct attributes on the user saved view' do + resolve_mutation(id: saved_view.to_global_id) + + user_saved_view = WorkItems::SavedViews::UserSavedView.last + + expect(user_saved_view.user).to eq(current_user) + expect(user_saved_view.saved_view).to eq(saved_view) + expect(user_saved_view.namespace).to eq(project.project_namespace) + end + + it 'sets the initial relative position' do + resolve_mutation(id: saved_view.to_global_id) + + user_saved_view = WorkItems::SavedViews::UserSavedView.last + + expect(user_saved_view.relative_position).not_to be_nil + end + + context 'when already subscribed' do + before do + create(:user_saved_view, user: current_user, saved_view: saved_view) + end + + it 'does not create a duplicate subscription' do + expect { resolve_mutation(id: saved_view.to_global_id) } + .not_to change { WorkItems::SavedViews::UserSavedView.count } + end + + it 'returns the saved view successfully' do + result = resolve_mutation(id: saved_view.to_global_id) + + expect(result[:saved_view]).to eq(saved_view) + expect(result[:errors]).to be_empty + end + end + + context 'when the subscription limit is reached' do + before do + # 5 subscriptions (the CE limit) + create_list(:saved_view, 5, namespace: project.project_namespace).each do |sv| + create(:user_saved_view, user: current_user, saved_view: sv) + end + end + + it 'returns an error' do + result = resolve_mutation(id: saved_view.to_global_id) + + expect(result[:saved_view]).to be_nil + expect(result[:errors]).to eq(['Saved view limit exceeded.']) + end + + it 'does not create a new subscription' do + expect { resolve_mutation(id: saved_view.to_global_id) } + .not_to change { WorkItems::SavedViews::UserSavedView.count } + end + end end end end diff --git a/spec/graphql/mutations/work_items/saved_views/unsubscribe_spec.rb b/spec/graphql/mutations/work_items/saved_views/unsubscribe_spec.rb index d41812b2de31cc25da8d97ccbcc243590ffe2b04..b82d9dae6340e8b98c8e16b17748ec15df1ca9b5 100644 --- a/spec/graphql/mutations/work_items/saved_views/unsubscribe_spec.rb +++ b/spec/graphql/mutations/work_items/saved_views/unsubscribe_spec.rb @@ -6,27 +6,90 @@ include GraphqlHelpers let_it_be(:project) { create(:project) } - let_it_be(:current_user) { create(:user) } + let_it_be(:current_user) { create(:user, planner_of: [project]) } let_it_be(:saved_view) { create(:saved_view, namespace: project.project_namespace) } - let(:arguments) { { id: saved_view.to_gid } } - subject(:mutation) { described_class.new(object: nil, context: query_context, field: nil) } + let(:current_ctx) { { current_user: current_user } } - context 'when the user is not logged in' do - let(:current_user) { nil } + def resolve_mutation(**args) + resolve(described_class, args: args, ctx: current_ctx) + end + + describe '#resolve' do + context 'when the user is not logged in' do + let(:current_ctx) { { current_user: nil } } - it 'raises an appropriate error' do - expect { mutation.resolve(**arguments) }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + it 'raises an appropriate error' do + expect_graphql_error_to_be_created(Gitlab::Graphql::Errors::ResourceNotAvailable) do + resolve_mutation(id: saved_view.to_global_id) + end + end end - end - context 'when the user is logged in and has planner permissions in the project' do - before_all do - project.add_planner(current_user) + context 'when the user is a guest' do + let_it_be(:guest_user) { create(:user) } + let(:current_ctx) { { current_user: guest_user } } + + before_all do + project.add_guest(guest_user) + end + + it 'allows unsubscription from the saved view' do + create(:user_saved_view, user: guest_user, saved_view: saved_view) + + result = resolve_mutation(id: saved_view.to_global_id) + + expect(result[:saved_view]).to eq(saved_view) + end end - it 'does not raise an error' do - expect { mutation.resolve(**arguments) }.not_to raise_error + context 'when saved views are not enabled for the namespace' do + before do + stub_feature_flags(work_items_saved_views: false) + end + + it 'returns an error' do + result = resolve_mutation(id: saved_view.to_global_id) + + expect(result[:saved_view]).to be_nil + expect(result[:errors]).to eq(['Saved views are not enabled for this namespace.']) + end + end + + context 'when the user has permission' do + context 'when subscribed to the saved view' do + let_it_be(:user_saved_view) { create(:user_saved_view, user: current_user, saved_view: saved_view) } + + it 'successfully unsubscribes from the saved view' do + result = resolve_mutation(id: saved_view.to_global_id) + + expect(result[:saved_view]).to eq(saved_view) + end + + it 'deletes the user saved view record' do + expect { resolve_mutation(id: saved_view.to_global_id) } + .to change { WorkItems::SavedViews::UserSavedView.count }.by(-1) + end + + it 'removes the specific subscription' do + resolve_mutation(id: saved_view.to_global_id) + + expect(WorkItems::SavedViews::UserSavedView.find_by(id: user_saved_view.id)).to be_nil + end + end + + context 'when not subscribed to the saved view' do + it 'returns the saved view successfully' do + result = resolve_mutation(id: saved_view.to_global_id) + + expect(result[:saved_view]).to eq(saved_view) + end + + it 'does not change the user saved view count' do + expect { resolve_mutation(id: saved_view.to_global_id) } + .not_to change { WorkItems::SavedViews::UserSavedView.count } + end + end end end end diff --git a/spec/graphql/types/work_items/saved_views/saved_view_type_spec.rb b/spec/graphql/types/work_items/saved_views/saved_view_type_spec.rb index e8ab3d73c6296346ebdec13c38d58a739d42eecd..ea311abdb3bbe82e047a6d975894595d7df5fb8c 100644 --- a/spec/graphql/types/work_items/saved_views/saved_view_type_spec.rb +++ b/spec/graphql/types/work_items/saved_views/saved_view_type_spec.rb @@ -6,34 +6,36 @@ include GraphqlHelpers let_it_be(:current_user) { create(:user) } + let_it_be(:group) { create(:group, planners: [current_user]) } + + let_it_be(:filter_data) { { state: 'opened', confidential: true } } + let_it_be(:display_settings) { { 'hiddenMetadataKeys' => ['assignee'] } } + + let_it_be(:saved_view) do + create(:saved_view, namespace: group, filter_data: filter_data, display_settings: display_settings, + sort: :priority_asc) + end specify { expect(described_class).to require_graphql_authorizations(:read_saved_view) } describe '#filters' do - it 'returns an empty hash' do - saved_view = create(:saved_view) - - expect(resolve_field(:filters, saved_view, current_user: current_user)).to eq({}) + it 'returns the specified filtering options' do + expect(resolve_field(:filters, saved_view, current_user: current_user)).to eq(filter_data) end end describe '#filter_warnings' do it 'returns an empty array' do - saved_view = create(:saved_view) - expect(resolve_field(:filter_warnings, saved_view, current_user: current_user)).to eq([]) end end describe '#share_url' do context 'when namespace is a group' do - let_it_be(:group) { create(:group, planners: [current_user]) } - let_it_be(:saved_view) { create(:saved_view, namespace: group) } - - it 'returns the group subscribe URL' do + it 'returns the group URL' do url = resolve_field(:share_url, saved_view, current_user: current_user) - expect(url).to eq(Gitlab::Routing.url_helpers.subscribe_group_saved_view_url(group, saved_view.id)) + expect(url).to eq(Gitlab::Routing.url_helpers.group_saved_view_url(group, saved_view.id)) end end @@ -45,10 +47,10 @@ project.add_planner(current_user) end - it 'returns the project subscribe URL' do + it 'returns the project URL' do url = resolve_field(:share_url, saved_view, current_user: current_user) - expect(url).to eq(Gitlab::Routing.url_helpers.subscribe_project_saved_view_url(project, saved_view.id)) + expect(url).to eq(Gitlab::Routing.url_helpers.project_saved_view_url(project, saved_view.id)) end end end diff --git a/spec/services/work_items/saved_views/reorder_service_spec.rb b/spec/services/work_items/saved_views/reorder_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..6f867b9e762c6f39fcdccfc95e71c9835cd177f6 --- /dev/null +++ b/spec/services/work_items/saved_views/reorder_service_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::SavedViews::ReorderService, feature_category: :portfolio_management do + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:namespace) { project.project_namespace } + + let_it_be(:sv_1) { create(:saved_view, namespace: namespace) } + let_it_be(:sv_2) { create(:saved_view, namespace: namespace) } + let_it_be(:sv_3) { create(:saved_view, namespace: namespace) } + let_it_be(:usv_1) { create(:user_saved_view, user: current_user, saved_view: sv_1, relative_position: 100) } + let_it_be(:usv_2) { create(:user_saved_view, user: current_user, saved_view: sv_2, relative_position: 200) } + let_it_be(:usv_3) { create(:user_saved_view, user: current_user, saved_view: sv_3, relative_position: 300) } + + let(:params) { {} } + + subject(:service) { described_class.new(current_user: current_user, params: params) } + + describe '#execute' do + context 'when moving a saved view before another' do + let(:params) { { move_before_id: sv_1.id } } + + it 'successfully reorders the saved view' do + result = service.execute(sv_3) + + expect(result).to be_success + expect(result.payload[:saved_view].saved_view).to eq(sv_3) + + user_saved_views = WorkItems::SavedViews::UserSavedView + .in_namespace(namespace) + .for_user(current_user) + .order(:relative_position) + + expect(user_saved_views.map(&:saved_view_id)).to eq([sv_3.id, sv_1.id, sv_2.id]) + end + + it 'updates the relative_position' do + expect { service.execute(sv_3) }.to change { usv_3.reload.relative_position }.from(300) + end + end + + context 'when moving a saved view after another' do + let(:params) { { move_after_id: sv_3.id } } + + it 'successfully reorders the saved view' do + result = service.execute(sv_1) + + expect(result).to be_success + expect(result.payload[:saved_view].saved_view).to eq(sv_1) + + user_saved_views = WorkItems::SavedViews::UserSavedView + .in_namespace(namespace) + .for_user(current_user) + .order(:relative_position) + + expect(user_saved_views.map(&:saved_view_id)).to eq([sv_2.id, sv_3.id, sv_1.id]) + end + end + + context 'when the user is not subscribed to the saved view being moved' do + let(:unsubscribed_sv) { create(:saved_view, namespace: namespace) } + let(:params) { { move_before_id: sv_1.id } } + + it 'returns an error' do + result = service.execute(unsubscribed_sv) + + expect(result).to be_error + expect(result.message).to eq("Unable to find subscribed saved view(s)") + end + end + + context 'when the user is not subscribed to the adjacent saved view' do + let(:unsubscribed_sv) { create(:saved_view, namespace: namespace) } + let(:params) { { move_before_id: unsubscribed_sv.id } } + + it 'returns an error' do + result = service.execute(sv_1) + + expect(result).to be_error + expect(result.message).to eq("Unable to find subscribed saved view(s)") + end + end + + context 'when neither move_before_id nor move_after_id is provided' do + let(:params) { {} } + + it 'returns an error' do + result = service.execute(sv_1) + + expect(result).to be_error + expect(result.message).to eq("Unable to find subscribed saved view(s)") + end + end + + context 'when the adjacent saved view is from a different namespace' do + let_it_be(:other_project) { create(:project) } + let_it_be(:other_sv) { create(:saved_view, namespace: other_project.project_namespace) } + let(:params) { { move_before_id: other_sv.id } } + + before_all do + create(:user_saved_view, user: current_user, saved_view: other_sv) + end + + it 'returns an error' do + result = service.execute(sv_1) + + expect(result).to be_error + expect(result.message).to eq("Unable to find subscribed saved view(s)") + end + end + end +end