diff --git a/.gitlab/issue_templates/Problem Validation.md b/.gitlab/issue_templates/Problem Validation.md index f7515c07218..5d417c5a26d 100644 --- a/.gitlab/issue_templates/Problem Validation.md +++ b/.gitlab/issue_templates/Problem Validation.md @@ -38,4 +38,11 @@ For example, if the solution will take a product manager, designer, and engineer two weeks of effort - you may quantify this as 1.5 (based on 0.5 months x 3 people). --> +## Definition of Done + +- [ ] The problem is well understood by the PM to have an understanding summarized in a RICE score +- [ ] The problem is well understood by the PM to decide if they want to move forward with this idea or drop it +- [ ] The problem is well described and detailed with necessary requirements for product design to understand the problem +- [ ] The problem is well described and detailed with necessary requirements for engineering to understand the problem + /label ~"workflow::validation backlog" ~devops:: ~category: ~group:: diff --git a/app/graphql/mutations/groups/update.rb b/app/graphql/mutations/groups/update.rb new file mode 100644 index 00000000000..f4eb94f9f84 --- /dev/null +++ b/app/graphql/mutations/groups/update.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Mutations + module Groups + class Update < Mutations::BaseMutation + include Mutations::ResolvesGroup + + graphql_name 'GroupUpdate' + + authorize :admin_group + + field :group, Types::GroupType, + null: true, + description: 'The group after update.' + + argument :full_path, GraphQL::Types::ID, + required: true, + description: 'Full path of the group that will be updated.' + argument :shared_runners_setting, Types::Namespace::SharedRunnersSettingEnum, + required: true, + description: copy_field_description(Types::GroupType, :shared_runners_setting) + + def resolve(full_path:, **args) + group = authorized_find!(full_path: full_path) + + unless ::Groups::UpdateService.new(group, current_user, args).execute + return { group: nil, errors: group.errors.full_messages } + end + + { group: group, errors: [] } + end + + private + + def find_object(full_path:) + resolve_group(full_path: full_path) + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 812943e0a1e..530298ba07a 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -105,6 +105,7 @@ module Types mount_mutation Mutations::Ci::Runner::Delete, feature_flag: :runner_graphql_query mount_mutation Mutations::Ci::RunnersRegistrationToken::Reset, feature_flag: :runner_graphql_query mount_mutation Mutations::Namespace::PackageSettings::Update + mount_mutation Mutations::Groups::Update mount_mutation Mutations::UserCallouts::Create mount_mutation Mutations::Packages::Destroy mount_mutation Mutations::Packages::DestroyFile diff --git a/config/initializers/00_rails_disable_joins.rb b/config/initializers/00_rails_disable_joins.rb new file mode 100644 index 00000000000..4274365ccad --- /dev/null +++ b/config/initializers/00_rails_disable_joins.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +# Backported from Rails 7.0 +# Initial support for has_many :through was implemented in https://github.com/rails/rails/pull/41937 +# Support for has_one :through was implemented in https://github.com/rails/rails/pull/42079 +raise 'DisableJoins patch is only to be used with versions of Rails < 7.0' unless Rails::VERSION::MAJOR < 7 + +ActiveRecord::Associations::Association.prepend(GemExtensions::ActiveRecord::Association) +# Temporarily allow :disable_joins to accept a lambda argument, to control rollout with feature flags +ActiveRecord::Associations::Association.prepend(GemExtensions::ActiveRecord::ConfigurableDisableJoins) +ActiveRecord::Associations::Builder::HasOne.prepend(GemExtensions::ActiveRecord::Associations::Builder::HasOne) +ActiveRecord::Associations::Builder::HasMany.prepend(GemExtensions::ActiveRecord::Associations::Builder::HasMany) +ActiveRecord::Associations::HasOneThroughAssociation.prepend(GemExtensions::ActiveRecord::Associations::HasOneThroughAssociation) +ActiveRecord::Associations::HasManyThroughAssociation.prepend(GemExtensions::ActiveRecord::Associations::HasManyThroughAssociation) +ActiveRecord::Associations::Preloader::ThroughAssociation.prepend(GemExtensions::ActiveRecord::Associations::Preloader::ThroughAssociation) +ActiveRecord::Base.extend(GemExtensions::ActiveRecord::DelegateCache) diff --git a/db/migrate/20210805085706_add_rule_index_to_security_orchestration_policy_rule_schedules.rb b/db/migrate/20210805085706_add_rule_index_to_security_orchestration_policy_rule_schedules.rb new file mode 100644 index 00000000000..ea77e29d365 --- /dev/null +++ b/db/migrate/20210805085706_add_rule_index_to_security_orchestration_policy_rule_schedules.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddRuleIndexToSecurityOrchestrationPolicyRuleSchedules < ActiveRecord::Migration[6.1] + def change + add_column :security_orchestration_policy_rule_schedules, :rule_index, :integer, null: false, default: 0 + end +end diff --git a/db/schema_migrations/20210805085706 b/db/schema_migrations/20210805085706 new file mode 100644 index 00000000000..b41a68968ec --- /dev/null +++ b/db/schema_migrations/20210805085706 @@ -0,0 +1 @@ +ec968f1f9fcc5a3551664e74726e1c65b327128e2388e1357ae6d0cf6f05fb95 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 189fe5fc5b0..688cd0f1452 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18053,6 +18053,7 @@ CREATE TABLE security_orchestration_policy_rule_schedules ( user_id bigint NOT NULL, policy_index integer NOT NULL, cron text NOT NULL, + rule_index integer DEFAULT 0 NOT NULL, CONSTRAINT check_915825a76e CHECK ((char_length(cron) <= 255)) ); diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 5e8e53f73c4..b52f560dc48 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -2360,6 +2360,26 @@ Input type: `GitlabSubscriptionActivateInput` | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | `license` | [`CurrentLicense`](#currentlicense) | The current license. | +### `Mutation.groupUpdate` + +Input type: `GroupUpdateInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `fullPath` | [`ID!`](#id) | Full path of the group that will be updated. | +| `sharedRunnersSetting` | [`SharedRunnersSetting!`](#sharedrunnerssetting) | Shared runners availability for the namespace and its descendants. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | +| `group` | [`Group`](#group) | The group after update. | + ### `Mutation.httpIntegrationCreate` Input type: `HttpIntegrationCreateInput` diff --git a/doc/development/documentation/feature_flags.md b/doc/development/documentation/feature_flags.md index d29f89e14cd..b0fa6c3428c 100644 --- a/doc/development/documentation/feature_flags.md +++ b/doc/development/documentation/feature_flags.md @@ -1,8 +1,6 @@ --- type: reference, dev -stage: none -group: Development -info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-development-guidelines" +info: For assistance with this Style Guide page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-other-projects-and-subjects. description: "GitLab development - how to document features deployed behind feature flags" --- @@ -30,31 +28,31 @@ Information about feature flags should be in a **Note** at the start of the topi The note has three parts, and follows this structure: ```markdown -NOTE: +FLAG: ``` ### Self-managed GitLab availability information -|If the feature is... | Use this text | -|-|-| -|Available|`On self-managed GitLab, by default this feature is available. To hide the feature, ask an administrator to [disable the flag](/administration/feature_flags.md).`| -|Unavailable|`On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the flag](/administration/feature_flags.md).`| -|Available, per-group|`On self-managed GitLab, by default this feature is available. To hide the feature per group, ask an administrator to [disable the flag](/administration/feature_flags.md).`| -|Unavailable, per-group|`On self-managed GitLab, by default this feature is not available. To make it available per group, ask an administrator to [enable the flag](/administration/feature_flags.md).`| -|Available, per-project|`On self-managed GitLab, by default this feature is available. To hide the feature per project or for your entire instance, ask an administrator to [disable the flag](/administration/feature_flags.md).`| -|Unavailable, per-project|`On self-managed GitLab, by default this feature is not available. To make it available per project or for your entire instance, ask an administrator to [enable the flag](/administration/feature_flags.md).`| -|Available, per-user|`On self-managed GitLab, by default this feature is available. To hide the feature per user, ask an administrator to [disable the flag](/administration/feature_flags.md).`| -|Unavailable, per-user|`On self-managed GitLab, by default this feature is not available. To make it available per user, ask an administrator to [enable the flag](/administration/feature_flags.md).`| +| If the feature is... | Use this text | +|--------------------------|---------------| +| Available | `On self-managed GitLab, by default this feature is available. To hide the feature, ask an administrator to [disable the flag](/administration/feature_flags.md).` | +| Unavailable | `On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the flag](/administration/feature_flags.md).` | +| Available, per-group | `On self-managed GitLab, by default this feature is available. To hide the feature per group, ask an administrator to [disable the flag](/administration/feature_flags.md).` | +| Unavailable, per-group | `On self-managed GitLab, by default this feature is not available. To make it available per group, ask an administrator to [enable the flag](/administration/feature_flags.md).` | +| Available, per-project | `On self-managed GitLab, by default this feature is available. To hide the feature per project or for your entire instance, ask an administrator to [disable the flag](/administration/feature_flags.md).` | +| Unavailable, per-project | `On self-managed GitLab, by default this feature is not available. To make it available per project or for your entire instance, ask an administrator to [enable the flag](/administration/feature_flags.md).` | +| Available, per-user | `On self-managed GitLab, by default this feature is available. To hide the feature per user, ask an administrator to [disable the flag](/administration/feature_flags.md).` | +| Unavailable, per-user | `On self-managed GitLab, by default this feature is not available. To make it available per user, ask an administrator to [enable the flag](/administration/feature_flags.md).` | ### GitLab.com availability information -|If the feature is... | Use this text | -|-|-| -|Available| `On GitLab.com, this feature is available.` | -|Available to GitLab.com admins only| `On GitLab.com, this feature is available but can be configured by GitLab.com administrators only.` -|Unavailable| `On GitLab.com, this feature is not available.`| +| If the feature is... | Use this text | +|-------------------------------------|---------------| +| Available | `On GitLab.com, this feature is available.` | +| Available to GitLab.com admins only | `On GitLab.com, this feature is available but can be configured by GitLab.com administrators only.` +| Unavailable | `On GitLab.com, this feature is not available.`| ### Optional information @@ -82,7 +80,7 @@ The following examples show the progression of a feature flag. ```markdown > Introduced in GitLab 13.7. -NOTE: +FLAG: On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the `forti_token_cloud` flag](../administration/feature_flags.md).` The feature is not ready for production use. @@ -94,7 +92,7 @@ If it were to be updated in the future to enable its use in production, you can > - Introduced in GitLab 13.7. > - [Enabled with `forti_token_cloud` flag](https://gitlab.com/issue/etc) for self-managed GitLab in GitLab X.X and ready for production use. -NOTE: +FLAG: On self-managed GitLab, by default this feature is available. To hide the feature per user, ask an administrator to [disable the `forti_token_cloud` flag](../administration/feature_flags.md). ``` diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md index 792b64c318b..6c05101436c 100644 --- a/doc/development/documentation/styleguide/index.md +++ b/doc/development/documentation/styleguide/index.md @@ -1337,8 +1337,13 @@ example: Use alert boxes to call attention to information. -Alert boxes are generated when the words `NOTE:` or `WARNING:` are followed by a -line break. For example: +Alert boxes are generated when one of these words is followed by a line break: + +- `FLAG:` +- `NOTE:` +- `WARNING:` + +For example: ```markdown NOTE: @@ -1351,6 +1356,11 @@ To display an alert box for multiple paragraphs, lists, or headers, use Alert boxes render only on the GitLab documentation site (). In the GitLab product help, alert boxes appear as plain text. +### Flag + +Use this alert type to describe a feature's availability. For information about how to format +`FLAG` alerts, see [Document features deployed behind feature flags](../feature_flags.md). + ### Note Use notes sparingly. Too many notes can make topics difficult to scan. diff --git a/doc/subscriptions/self_managed/index.md b/doc/subscriptions/self_managed/index.md index 027866ceb5e..5fd11a50a8d 100644 --- a/doc/subscriptions/self_managed/index.md +++ b/doc/subscriptions/self_managed/index.md @@ -38,14 +38,23 @@ for each tier, see the ## Subscription seats A GitLab self-managed subscription uses a hybrid model. You pay for a subscription -according to the maximum number of users enabled during the subscription period. +according to the [maximum number](#maximum-users) of users enabled during the subscription period. For instances that aren't offline or on a closed network, the maximum number of simultaneous users in the GitLab self-managed installation is checked each quarter. -If an instance is unable to generate a quarterly usage report, the existing [true-up model](#users-over-license) is used. +If an instance is unable to generate a quarterly usage report, the existing [true-up model](#users-over-license) is used. Prorated charges are not possible without a quarterly usage report. -### Billable users +### View user totals + +You can view users for your license and determine if you've gone over your subscription. + +1. On the top bar, select **Menu >** **{admin}** **Admin**. +1. On the left menu, select **Subscription**. + +The lists of users are displayed. + +#### Billable users A _billable user_ counts against the number of subscription seats. Every user is considered a billable user, with the following exceptions: @@ -63,11 +72,28 @@ billable user, with the following exceptions: **Billable users** as reported in the `/admin` section is updated once per day. -### Maximum users +#### Maximum users -GitLab shows the highest number of billable users for the current license period. +The number of _maximum users_ reflects the highest number of billable users for the current license period. -To view this list, on the top bar, select **Menu >** **{admin}** **Admin**. On the left menu, select **Subscription**. In the lower left, the list of **Maximum users** is displayed. +#### Users over license + +The number of _users over license_ shows how many users are in excess of the number allowed by the license. This number reflects the current license period. + +For example, if: + +- The license allows 100 users and +- **Maximum users** is 150, + +Then this value would be 50. + +If the **Maximum users** value is less than or equal to 100, then this value is 0. + +A trial license always displays zero for **Users over license**. + +If you add more users to your GitLab instance than you are licensed for, payment for the additional users is due [at the time of renewal](../quarterly_reconciliation.md). + +If you do not add these users during the renewal process, your license key will not work. ### Tips for managing users and subscription seats diff --git a/doc/user/admin_area/settings/help_page.md b/doc/user/admin_area/settings/help_page.md index 1106425d64e..1dab2e63c84 100644 --- a/doc/user/admin_area/settings/help_page.md +++ b/doc/user/admin_area/settings/help_page.md @@ -71,7 +71,7 @@ You can specify a custom URL to which users are directed when they: > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/43157) in GitLab 13.5. > - Enabled on GitLab.com and is ready for production use. -NOTE: +FLAG: On self-managed GitLab, by default this feature is not available. To make it available, ask an administrator to [enable the `:help_page_documentation_redirect` flag](../../../administration/feature_flags.md). On GitLab.com, this feature is available but can be configured by GitLab.com administrators only. diff --git a/lib/gem_extensions/active_record/association.rb b/lib/gem_extensions/active_record/association.rb new file mode 100644 index 00000000000..91a9f45ce7e --- /dev/null +++ b/lib/gem_extensions/active_record/association.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module GemExtensions + module ActiveRecord + module Association + extend ActiveSupport::Concern + + attr_reader :disable_joins + + def initialize(owner, reflection) + super + + @disable_joins = @reflection.options[:disable_joins] || false + end + + def scope + if disable_joins + DisableJoins::Associations::AssociationScope.create.scope(self) + else + super + end + end + + def association_scope + if klass + @association_scope ||= begin # rubocop:disable Gitlab/ModuleWithInstanceVariables + if disable_joins + DisableJoins::Associations::AssociationScope.scope(self) + else + super + end + end + end + end + end + end +end diff --git a/lib/gem_extensions/active_record/associations/builder/has_many.rb b/lib/gem_extensions/active_record/associations/builder/has_many.rb new file mode 100644 index 00000000000..7e51e632cc3 --- /dev/null +++ b/lib/gem_extensions/active_record/associations/builder/has_many.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module GemExtensions + module ActiveRecord + module Associations + module Builder + module HasMany + extend ActiveSupport::Concern + + class_methods do + def valid_options(options) + valid = super + valid += [:disable_joins] if options[:disable_joins] && options[:through] + valid + end + end + end + end + end + end +end diff --git a/lib/gem_extensions/active_record/associations/builder/has_one.rb b/lib/gem_extensions/active_record/associations/builder/has_one.rb new file mode 100644 index 00000000000..91765db8a5a --- /dev/null +++ b/lib/gem_extensions/active_record/associations/builder/has_one.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module GemExtensions + module ActiveRecord + module Associations + module Builder + module HasOne + extend ActiveSupport::Concern + + class_methods do + def valid_options(options) + valid = super + valid += [:disable_joins] if options[:disable_joins] && options[:through] + valid + end + end + end + end + end + end +end diff --git a/lib/gem_extensions/active_record/associations/has_many_through_association.rb b/lib/gem_extensions/active_record/associations/has_many_through_association.rb new file mode 100644 index 00000000000..e7051e4d9cb --- /dev/null +++ b/lib/gem_extensions/active_record/associations/has_many_through_association.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module GemExtensions + module ActiveRecord + module Associations + module HasManyThroughAssociation + extend ActiveSupport::Concern + + def find_target + return [] unless target_reflection_has_associated_record? + return scope.to_a if disable_joins + + super + end + end + end + end +end diff --git a/lib/gem_extensions/active_record/associations/has_one_through_association.rb b/lib/gem_extensions/active_record/associations/has_one_through_association.rb new file mode 100644 index 00000000000..1487392a4ea --- /dev/null +++ b/lib/gem_extensions/active_record/associations/has_one_through_association.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module GemExtensions + module ActiveRecord + module Associations + module HasOneThroughAssociation + extend ActiveSupport::Concern + + def find_target + return scope.first if disable_joins + + super + end + end + end + end +end diff --git a/lib/gem_extensions/active_record/associations/preloader/through_association.rb b/lib/gem_extensions/active_record/associations/preloader/through_association.rb new file mode 100644 index 00000000000..16b53846a58 --- /dev/null +++ b/lib/gem_extensions/active_record/associations/preloader/through_association.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module GemExtensions + module ActiveRecord + module Associations + module Preloader + module ThroughAssociation + extend ActiveSupport::Concern + + def through_scope + scope = through_reflection.klass.unscoped + options = reflection.options + + return scope if options[:disable_joins] + + super + end + end + end + end + end +end diff --git a/lib/gem_extensions/active_record/configurable_disable_joins.rb b/lib/gem_extensions/active_record/configurable_disable_joins.rb new file mode 100644 index 00000000000..8e4c6bd6fc5 --- /dev/null +++ b/lib/gem_extensions/active_record/configurable_disable_joins.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module GemExtensions + module ActiveRecord + module ConfigurableDisableJoins + extend ActiveSupport::Concern + + def disable_joins + # rubocop:disable Gitlab/ModuleWithInstanceVariables + return @disable_joins.call if @disable_joins.is_a?(Proc) + + @disable_joins + # rubocop:enable Gitlab/ModuleWithInstanceVariables + end + end + end +end diff --git a/lib/gem_extensions/active_record/delegate_cache.rb b/lib/gem_extensions/active_record/delegate_cache.rb new file mode 100644 index 00000000000..acdf1b6cca7 --- /dev/null +++ b/lib/gem_extensions/active_record/delegate_cache.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module GemExtensions + module ActiveRecord + module DelegateCache + def relation_delegate_class(klass) + @relation_delegate_cache2[klass] || super # rubocop:disable Gitlab/ModuleWithInstanceVariables + end + + def initialize_relation_delegate_cache_disable_joins + @relation_delegate_cache2 = {} # rubocop:disable Gitlab/ModuleWithInstanceVariables + + [ + DisableJoins::Relation + ].each do |klass| + delegate = Class.new(klass) do + include ::ActiveRecord::Delegation::ClassSpecificRelation + end + include_relation_methods(delegate) + mangled_name = klass.name.gsub("::", "_") + const_set mangled_name, delegate + private_constant mangled_name + + @relation_delegate_cache2[klass] = delegate # rubocop:disable Gitlab/ModuleWithInstanceVariables + end + end + + def inherited(child_class) + child_class.initialize_relation_delegate_cache_disable_joins + super + end + end + end +end diff --git a/lib/gem_extensions/active_record/disable_joins/associations/association_scope.rb b/lib/gem_extensions/active_record/disable_joins/associations/association_scope.rb new file mode 100644 index 00000000000..1e4476330a2 --- /dev/null +++ b/lib/gem_extensions/active_record/disable_joins/associations/association_scope.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +module GemExtensions + module ActiveRecord + module DisableJoins + module Associations + class AssociationScope < ::ActiveRecord::Associations::AssociationScope # :nodoc: + def scope(association) + source_reflection = association.reflection + owner = association.owner + unscoped = association.klass.unscoped + reverse_chain = get_chain(source_reflection, association, unscoped.alias_tracker).reverse + + previous_reflection, last_reflection, last_ordered, last_join_ids = last_scope_chain(reverse_chain, owner) + + add_constraints(last_reflection, last_reflection.join_primary_key, last_join_ids, owner, last_ordered, + previous_reflection: previous_reflection) + end + + private + + def last_scope_chain(reverse_chain, owner) + # Pulled from https://github.com/rails/rails/pull/42448 + # Fixes cases where the foreign key is not id + first_item = reverse_chain.shift + first_scope = [nil, first_item, false, [owner._read_attribute(first_item.join_foreign_key)]] + + reverse_chain.inject(first_scope) do |(previous_reflection, reflection, ordered, join_ids), next_reflection| + key = reflection.join_primary_key + records = add_constraints(reflection, key, join_ids, owner, ordered, previous_reflection: previous_reflection) + foreign_key = next_reflection.join_foreign_key + record_ids = records.pluck(foreign_key) # rubocop:disable CodeReuse/ActiveRecord + records_ordered = records && records.order_values.any? + + [reflection, next_reflection, records_ordered, record_ids] + end + end + + def add_constraints(reflection, key, join_ids, owner, ordered, previous_reflection: nil) + scope = reflection.build_scope(reflection.aliased_table).where(key => join_ids) # rubocop:disable CodeReuse/ActiveRecord + + # Pulled from https://github.com/rails/rails/pull/42590 + # Fixes cases where used with an STI type + relation = reflection.klass.scope_for_association + scope.merge!( + relation.except(:select, :create_with, :includes, :preload, :eager_load, :joins, :left_outer_joins) + ) + + # Attempt to fix use case where we have a polymorphic relationship + # Build on an additional scope to filter by the polymorphic type + if reflection.type + polymorphic_class = previous_reflection.try(:klass) || owner.class + + polymorphic_type = transform_value(polymorphic_class.polymorphic_name) + scope = apply_scope(scope, reflection.aliased_table, reflection.type, polymorphic_type) + end + + scope = reflection.constraints.inject(scope) do |memo, scope_chain_item| + item = eval_scope(reflection, scope_chain_item, owner) + scope.unscope!(*item.unscope_values) + scope.where_clause += item.where_clause + scope.order_values = item.order_values | scope.order_values + scope + end + + if scope.order_values.empty? && ordered + split_scope = DisableJoins::Relation.create(scope.klass, key, join_ids) + split_scope.where_clause += scope.where_clause + split_scope + else + scope + end + end + end + end + end + end +end diff --git a/lib/gem_extensions/active_record/disable_joins/relation.rb b/lib/gem_extensions/active_record/disable_joins/relation.rb new file mode 100644 index 00000000000..01eb6381a85 --- /dev/null +++ b/lib/gem_extensions/active_record/disable_joins/relation.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module GemExtensions + module ActiveRecord + module DisableJoins + class Relation < ::ActiveRecord::Relation + attr_reader :ids, :key + + def initialize(klass, key, ids) + @ids = ids.uniq + @key = key + super(klass) + end + + def limit(value) + records.take(value) # rubocop:disable CodeReuse/ActiveRecord + end + + def first(limit = nil) + if limit + records.limit(limit).first + else + records.first + end + end + + def load + super + records = @records + + records_by_id = records.group_by do |record| + record[key] + end + + records = ids.flat_map { |id| records_by_id[id.to_i] } + records.compact! + + @records = records + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d05466e9b9b..3f985b32a1e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5265,6 +5265,9 @@ msgstr "" msgid "Billing|Enter at least three characters to search." msgstr "" +msgid "Billing|Export list" +msgstr "" + msgid "Billing|Group" msgstr "" @@ -13621,6 +13624,9 @@ msgstr "" msgid "Failed to find import label for Jira import." msgstr "" +msgid "Failed to generate export, please try again later." +msgstr "" + msgid "Failed to generate report, please try again after sometime" msgstr "" diff --git a/spec/graphql/mutations/groups/update_spec.rb b/spec/graphql/mutations/groups/update_spec.rb new file mode 100644 index 00000000000..2118134e8e6 --- /dev/null +++ b/spec/graphql/mutations/groups/update_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::Groups::Update do + using RSpec::Parameterized::TableSyntax + + let_it_be_with_reload(:group) { create(:group) } + let_it_be(:user) { create(:user) } + + let(:params) { { full_path: group.full_path } } + + specify { expect(described_class).to require_graphql_authorizations(:admin_group) } + + describe '#resolve' do + subject { described_class.new(object: group, context: { current_user: user }, field: nil).resolve(**params) } + + RSpec.shared_examples 'updating the group shared runners setting' do + it 'updates the group shared runners setting' do + expect { subject } + .to change { group.reload.shared_runners_setting }.from('enabled').to('disabled_and_unoverridable') + end + + it 'returns no errors' do + expect(subject).to eq(errors: [], group: group) + end + + context 'with invalid params' do + let_it_be(:params) { { full_path: group.full_path, shared_runners_setting: 'inexistent_setting' } } + + it 'doesn\'t update the shared_runners_setting' do + expect { subject } + .not_to change { group.reload.shared_runners_setting } + end + + it 'returns an error' do + expect(subject).to eq( + group: nil, + errors: ["Update shared runners state must be one of: #{::Namespace::SHARED_RUNNERS_SETTINGS.join(', ')}"] + ) + end + end + end + + RSpec.shared_examples 'denying access to group shared runners setting' do + it 'raises Gitlab::Graphql::Errors::ResourceNotAvailable' do + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) + end + end + + context 'changing shared runners setting' do + let_it_be(:params) do + { full_path: group.full_path, + shared_runners_setting: 'disabled_and_unoverridable' } + end + + where(:user_role, :shared_examples_name) do + :owner | 'updating the group shared runners setting' + :developer | 'denying access to group shared runners setting' + :reporter | 'denying access to group shared runners setting' + :guest | 'denying access to group shared runners setting' + :anonymous | 'denying access to group shared runners setting' + end + + with_them do + before do + group.send("add_#{user_role}", user) unless user_role == :anonymous + end + + it_behaves_like params[:shared_examples_name] + end + end + end +end diff --git a/spec/initializers/00_rails_disable_joins_spec.rb b/spec/initializers/00_rails_disable_joins_spec.rb new file mode 100644 index 00000000000..78e78b6810b --- /dev/null +++ b/spec/initializers/00_rails_disable_joins_spec.rb @@ -0,0 +1,288 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'DisableJoins' do + let(:primary_model) do + Class.new(ApplicationRecord) do + self.table_name = '_test_primary_records' + + def self.name + 'TestPrimary' + end + end + end + + let(:bridge_model) do + Class.new(ApplicationRecord) do + self.table_name = '_test_bridge_records' + + def self.name + 'TestBridge' + end + end + end + + let(:secondary_model) do + Class.new(ApplicationRecord) do + self.table_name = '_test_secondary_records' + + def self.name + 'TestSecondary' + end + end + end + + context 'passing disable_joins as an association option' do + context 'when the association is a bare has_one' do + it 'disallows the disable_joins option' do + expect do + primary_model.has_one :test_bridge, disable_joins: true + end.to raise_error(ArgumentError, /Unknown key: :disable_joins/) + end + end + + context 'when the association is a belongs_to' do + it 'disallows the disable_joins option' do + expect do + bridge_model.belongs_to :test_secondary, disable_joins: true + end.to raise_error(ArgumentError, /Unknown key: :disable_joins/) + end + end + + context 'when the association is has_one :through' do + it 'allows the disable_joins option' do + primary_model.has_one :test_bridge + bridge_model.belongs_to :test_secondary + + expect do + primary_model.has_one :test_secondary, through: :test_bridge, disable_joins: true + end.not_to raise_error + end + end + + context 'when the association is a bare has_many' do + it 'disallows the disable_joins option' do + expect do + primary_model.has_many :test_bridges, disable_joins: true + end.to raise_error(ArgumentError, /Unknown key: :disable_joins/) + end + end + + context 'when the association is a has_many :through' do + it 'allows the disable_joins option' do + primary_model.has_many :test_bridges + bridge_model.belongs_to :test_secondary + + expect do + primary_model.has_many :test_secondaries, through: :test_bridges, disable_joins: true + end.not_to raise_error + end + end + end + + context 'querying has_one :through when disable_joins is set' do + before do + create_tables(<<~SQL) + CREATE TABLE _test_primary_records ( + id serial NOT NULL PRIMARY KEY); + + CREATE TABLE _test_bridge_records ( + id serial NOT NULL PRIMARY KEY, + primary_record_id int NOT NULL, + secondary_record_id int NOT NULL); + + CREATE TABLE _test_secondary_records ( + id serial NOT NULL PRIMARY KEY); + SQL + + primary_model.has_one :test_bridge, anonymous_class: bridge_model, foreign_key: :primary_record_id + bridge_model.belongs_to :test_secondary, anonymous_class: secondary_model, foreign_key: :secondary_record_id + primary_model.has_one :test_secondary, through: :test_bridge, anonymous_class: secondary_model, + disable_joins: -> { joins_disabled_flag } + + primary_record = primary_model.create! + secondary_record = secondary_model.create! + bridge_model.create!(primary_record_id: primary_record.id, secondary_record_id: secondary_record.id) + end + + context 'when disable_joins evaluates to true' do + let(:joins_disabled_flag) { true } + + it 'executes separate queries' do + primary_record = primary_model.first + + query_count = ActiveRecord::QueryRecorder.new { primary_record.test_secondary }.count + + expect(query_count).to eq(2) + end + end + + context 'when disable_joins evalutes to false' do + let(:joins_disabled_flag) { false } + + it 'executes a single query' do + primary_record = primary_model.first + + query_count = ActiveRecord::QueryRecorder.new { primary_record.test_secondary }.count + + expect(query_count).to eq(1) + end + end + end + + context 'querying has_many :through when disable_joins is set' do + before do + create_tables(<<~SQL) + CREATE TABLE _test_primary_records ( + id serial NOT NULL PRIMARY KEY); + + CREATE TABLE _test_bridge_records ( + id serial NOT NULL PRIMARY KEY, + primary_record_id int NOT NULL); + + CREATE TABLE _test_secondary_records ( + id serial NOT NULL PRIMARY KEY, + bridge_record_id int NOT NULL); + SQL + + primary_model.has_many :test_bridges, anonymous_class: bridge_model, foreign_key: :primary_record_id + bridge_model.has_many :test_secondaries, anonymous_class: secondary_model, foreign_key: :bridge_record_id + primary_model.has_many :test_secondaries, through: :test_bridges, anonymous_class: secondary_model, + disable_joins: -> { disabled_join_flag } + + primary_record = primary_model.create! + bridge_record = bridge_model.create!(primary_record_id: primary_record.id) + secondary_model.create!(bridge_record_id: bridge_record.id) + end + + context 'when disable_joins evaluates to true' do + let(:disabled_join_flag) { true } + + it 'executes separate queries' do + primary_record = primary_model.first + + query_count = ActiveRecord::QueryRecorder.new { primary_record.test_secondaries.first }.count + + expect(query_count).to eq(2) + end + end + + context 'when disable_joins evalutes to false' do + let(:disabled_join_flag) { false } + + it 'executes a single query' do + primary_record = primary_model.first + + query_count = ActiveRecord::QueryRecorder.new { primary_record.test_secondaries.first }.count + + expect(query_count).to eq(1) + end + end + end + + context 'querying STI relationships' do + let(:child_bridge_model) do + Class.new(bridge_model) do + def self.name + 'ChildBridge' + end + end + end + + let(:child_secondary_model) do + Class.new(secondary_model) do + def self.name + 'ChildSecondary' + end + end + end + + before do + create_tables(<<~SQL) + CREATE TABLE _test_primary_records ( + id serial NOT NULL PRIMARY KEY); + + CREATE TABLE _test_bridge_records ( + id serial NOT NULL PRIMARY KEY, + primary_record_id int NOT NULL, + type text); + + CREATE TABLE _test_secondary_records ( + id serial NOT NULL PRIMARY KEY, + bridge_record_id int NOT NULL, + type text); + SQL + + primary_model.has_many :child_bridges, anonymous_class: child_bridge_model, foreign_key: :primary_record_id + child_bridge_model.has_one :child_secondary, anonymous_class: child_secondary_model, foreign_key: :bridge_record_id + primary_model.has_many :child_secondaries, through: :child_bridges, anonymous_class: child_secondary_model, disable_joins: true + + primary_record = primary_model.create! + parent_bridge_record = bridge_model.create!(primary_record_id: primary_record.id) + child_bridge_record = child_bridge_model.create!(primary_record_id: primary_record.id) + + secondary_model.create!(bridge_record_id: child_bridge_record.id) + child_secondary_model.create!(bridge_record_id: parent_bridge_record.id) + child_secondary_model.create!(bridge_record_id: child_bridge_record.id) + end + + it 'filters correctly by the STI type across multiple queries' do + primary_record = primary_model.first + + query_recorder = ActiveRecord::QueryRecorder.new do + expect(primary_record.child_secondaries.count).to eq(1) + end + + expect(query_recorder.count).to eq(2) + end + end + + context 'querying polymorphic relationships' do + before do + create_tables(<<~SQL) + CREATE TABLE _test_primary_records ( + id serial NOT NULL PRIMARY KEY); + + CREATE TABLE _test_bridge_records ( + id serial NOT NULL PRIMARY KEY, + primaryable_id int NOT NULL, + primaryable_type text NOT NULL); + + CREATE TABLE _test_secondary_records ( + id serial NOT NULL PRIMARY KEY, + bridgeable_id int NOT NULL, + bridgeable_type text NOT NULL); + SQL + + primary_model.has_many :test_bridges, anonymous_class: bridge_model, foreign_key: :primaryable_id, as: :primaryable + bridge_model.has_one :test_secondaries, anonymous_class: secondary_model, foreign_key: :bridgeable_id, as: :bridgeable + primary_model.has_many :test_secondaries, through: :test_bridges, anonymous_class: secondary_model, disable_joins: true + + primary_record = primary_model.create! + primary_bridge_record = bridge_model.create!(primaryable_id: primary_record.id, primaryable_type: 'TestPrimary') + nonprimary_bridge_record = bridge_model.create!(primaryable_id: primary_record.id, primaryable_type: 'NonPrimary') + + secondary_model.create!(bridgeable_id: primary_bridge_record.id, bridgeable_type: 'TestBridge') + secondary_model.create!(bridgeable_id: nonprimary_bridge_record.id, bridgeable_type: 'TestBridge') + secondary_model.create!(bridgeable_id: primary_bridge_record.id, bridgeable_type: 'NonBridge') + end + + it 'filters correctly by the polymorphic type across multiple queries' do + primary_record = primary_model.first + + query_recorder = ActiveRecord::QueryRecorder.new do + expect(primary_record.test_secondaries.count).to eq(1) + end + + expect(query_recorder.count).to eq(2) + end + end + + def create_tables(table_sql) + ApplicationRecord.connection.execute(table_sql) + + bridge_model.reset_column_information + secondary_model.reset_column_information + end +end diff --git a/spec/requests/api/graphql/mutations/groups/update_spec.rb b/spec/requests/api/graphql/mutations/groups/update_spec.rb new file mode 100644 index 00000000000..b9dfb8e37ab --- /dev/null +++ b/spec/requests/api/graphql/mutations/groups/update_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'GroupUpdate' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:group) { create(:group) } + + let(:variables) do + { + full_path: group.full_path, + shared_runners_setting: 'DISABLED_WITH_OVERRIDE' + } + end + + let(:mutation) { graphql_mutation(:group_update, variables) } + + context 'when unauthorized' do + shared_examples 'unauthorized' do + it 'returns an error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).not_to be_empty + end + end + + context 'when not a group member' do + it_behaves_like 'unauthorized' + end + + context 'when a non-admin group member' do + before do + group.add_developer(user) + end + + it_behaves_like 'unauthorized' + end + end + + context 'when authorized' do + before do + group.add_owner(user) + end + + it 'updates shared runners settings' do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_nil + expect(group.reload.shared_runners_setting).to eq(variables[:shared_runners_setting].downcase) + end + + context 'when bad arguments are provided' do + let(:variables) { { full_path: '', shared_runners_setting: 'INVALID' } } + + it 'returns the errors' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).not_to be_empty + expect(group.reload.shared_runners_setting).to eq('enabled') + end + end + end +end