diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-04-22 18:09:27 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-04-22 18:09:27 +0300 |
commit | 7a7345366550f509c03595e0dada7cbd0d73103d (patch) | |
tree | ae2c4e74faa9f87237082abf4b23f325e34df6e3 | |
parent | ae96e65ee23d81be3924b87ed16becbbbe002b91 (diff) |
Add latest changes from gitlab-org/gitlab@master
40 files changed, 492 insertions, 166 deletions
diff --git a/.gitlab/merge_request_templates/Documentation.md b/.gitlab/merge_request_templates/Documentation.md index ddba36efc17..7e9a0b7133a 100644 --- a/.gitlab/merge_request_templates/Documentation.md +++ b/.gitlab/merge_request_templates/Documentation.md @@ -29,7 +29,7 @@ When applicable: - [ ] Link docs to and from the higher-level index page, plus other related docs where helpful. - [ ] Add [GitLab's version history note(s)](https://docs.gitlab.com/ee/development/documentation/styleguide.html#text-for-documentation-requiring-version-text). - [ ] Add the [product tier badge](https://docs.gitlab.com/ee/development/documentation/styleguide.html#product-badges). -- [ ] Add/update the [feature flag section](https://docs.gitlab.com/ee/development/documentation/styleguide.html#feature-flags). +- [ ] Add/update the [feature flag section](https://docs.gitlab.com/ee/development/documentation/feature_flags.html). - [ ] If you're changing document headings, search `doc/*`, `app/views/*`, and `ee/app/views/*` for old headings replacing with the new ones to [avoid broken anchors](https://docs.gitlab.com/ee/development/documentation/styleguide.html#anchor-links). ## Review checklist diff --git a/app/assets/javascripts/blob/components/blob_header.vue b/app/assets/javascripts/blob/components/blob_header.vue index b7d9600ec40..e5e01caa9a5 100644 --- a/app/assets/javascripts/blob/components/blob_header.vue +++ b/app/assets/javascripts/blob/components/blob_header.vue @@ -66,7 +66,7 @@ export default { </template> </blob-filepath> - <div class="file-actions d-none d-sm-block"> + <div class="file-actions d-none d-sm-flex"> <viewer-switcher v-if="showViewerSwitcher" v-model="viewer" /> <slot name="actions"></slot> diff --git a/app/assets/javascripts/blob/components/blob_header_viewer_switcher.vue b/app/assets/javascripts/blob/components/blob_header_viewer_switcher.vue index 7155a1d35b1..ed03213d7cf 100644 --- a/app/assets/javascripts/blob/components/blob_header_viewer_switcher.vue +++ b/app/assets/javascripts/blob/components/blob_header_viewer_switcher.vue @@ -45,7 +45,7 @@ export default { }; </script> <template> - <gl-button-group class="js-blob-viewer-switcher ml-2"> + <gl-button-group class="js-blob-viewer-switcher mx-2"> <gl-deprecated-button v-gl-tooltip.hover :aria-label="$options.SIMPLE_BLOB_VIEWER_TITLE" diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb index f5785000062..8c9ad343f32 100644 --- a/app/models/ci/pipeline_schedule.rb +++ b/app/models/ci/pipeline_schedule.rb @@ -6,6 +6,10 @@ module Ci include Importable include StripAttribute include Schedulable + include Limitable + + self.limit_name = 'ci_pipeline_schedules' + self.limit_scope = :project belongs_to :project belongs_to :owner, class_name: 'User' diff --git a/app/models/concerns/limitable.rb b/app/models/concerns/limitable.rb new file mode 100644 index 00000000000..f320f54bb82 --- /dev/null +++ b/app/models/concerns/limitable.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Limitable + extend ActiveSupport::Concern + + included do + class_attribute :limit_scope + class_attribute :limit_name + self.limit_name = self.name.demodulize.tableize + + validate :validate_plan_limit_not_exceeded, on: :create + end + + private + + def validate_plan_limit_not_exceeded + scope_relation = self.public_send(limit_scope) # rubocop:disable GitlabSecurity/PublicSend + return unless scope_relation + + relation = self.class.where(limit_scope => scope_relation) + + if scope_relation.actual_limits.exceeded?(limit_name, relation) + errors.add(:base, _("Maximum number of %{name} (%{count}) exceeded") % + { name: limit_name.humanize(capitalize: false), count: scope_relation.actual_limits.public_send(limit_name) }) # rubocop:disable GitlabSecurity/PublicSend + end + end +end diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index bc480b14e67..71494b6de4d 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -3,6 +3,9 @@ class ProjectHook < WebHook include TriggerableHooks include Presentable + include Limitable + + self.limit_scope = :project triggerable_hooks [ :push_hooks, diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 9e7589a1f18..d8a305b4cf3 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -346,6 +346,21 @@ class Namespace < ApplicationRecord .try(name) end + def actual_plan + Plan.default + end + + def actual_limits + # We default to PlanLimits.new otherwise a lot of specs would fail + # On production each plan should already have associated limits record + # https://gitlab.com/gitlab-org/gitlab/issues/36037 + actual_plan.limits || PlanLimits.new + end + + def actual_plan_name + actual_plan.name + end + private def all_projects_with_pages diff --git a/app/models/plan.rb b/app/models/plan.rb new file mode 100644 index 00000000000..5cfa1e258d6 --- /dev/null +++ b/app/models/plan.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +class Plan < ApplicationRecord + DEFAULT = 'default'.freeze + + has_one :limits, class_name: 'PlanLimits' + + ALL_PLANS = [DEFAULT].freeze + DEFAULT_PLANS = [DEFAULT].freeze + private_constant :ALL_PLANS, :DEFAULT_PLANS + + # This always returns an object + def self.default + Gitlab::SafeRequestStore.fetch(:plan_default) do + # find_by allows us to find object (cheaply) against replica DB + # safe_find_or_create_by does stick to primary DB + find_by(name: DEFAULT) || safe_find_or_create_by(name: DEFAULT) + end + end + + def self.all_plans + ALL_PLANS + end + + def self.default_plans + DEFAULT_PLANS + end + + def default? + self.class.default_plans.include?(name) + end + + def paid? + false + end +end + +Plan.prepend_if_ee('EE::Plan') diff --git a/app/models/plan_limits.rb b/app/models/plan_limits.rb new file mode 100644 index 00000000000..7f5c9b8b6b4 --- /dev/null +++ b/app/models/plan_limits.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class PlanLimits < ApplicationRecord + belongs_to :plan + + def exceeded?(limit_name, object) + return false unless enabled?(limit_name) + + if object.is_a?(Integer) + object >= read_attribute(limit_name) + else + # object.count >= limit value is slower than checking + # if a record exists at the limit value - 1 position. + object.limit(1).offset(read_attribute(limit_name) - 1).exists? + end + end + + private + + def enabled?(limit_name) + read_attribute(limit_name) > 0 + end +end diff --git a/app/models/project.rb b/app/models/project.rb index d5c5ab99611..ab5d233060e 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -357,6 +357,7 @@ class Project < ApplicationRecord delegate :external_dashboard_url, to: :metrics_setting, allow_nil: true, prefix: true delegate :default_git_depth, :default_git_depth=, to: :ci_cd_settings, prefix: :ci delegate :forward_deployment_enabled, :forward_deployment_enabled=, :forward_deployment_enabled?, to: :ci_cd_settings + delegate :actual_limits, :actual_plan_name, to: :namespace, allow_nil: true # Validations validates :creator, presence: true, on: :create diff --git a/app/services/projects/propagate_service_template.rb b/app/services/projects/propagate_service_template.rb index 6013b00b8c6..0483c951f1e 100644 --- a/app/services/projects/propagate_service_template.rb +++ b/app/services/projects/propagate_service_template.rb @@ -4,8 +4,10 @@ module Projects class PropagateServiceTemplate BATCH_SIZE = 100 - def self.propagate(*args) - new(*args).propagate + delegate :data_fields_present?, to: :template + + def self.propagate(template) + new(template).propagate end def initialize(template) @@ -13,15 +15,15 @@ module Projects end def propagate - return unless @template.active? - - Rails.logger.info("Propagating services for template #{@template.id}") # rubocop:disable Gitlab/RailsLogger + return unless template.active? propagate_projects_with_template end private + attr_reader :template + def propagate_projects_with_template loop do batch = Project.uncached { project_ids_batch } @@ -38,7 +40,14 @@ module Projects end Project.transaction do - bulk_insert_services(service_hash.keys << 'project_id', service_list) + results = bulk_insert(Service, service_hash.keys << 'project_id', service_list) + + if data_fields_present? + data_list = results.map { |row| data_hash.values << row['id'] } + + bulk_insert(template.data_fields.class, data_hash.keys << 'service_id', data_list) + end + run_callbacks(batch) end end @@ -52,36 +61,27 @@ module Projects SELECT true FROM services WHERE services.project_id = projects.id - AND services.type = '#{@template.type}' + AND services.type = #{ActiveRecord::Base.connection.quote(template.type)} ) AND projects.pending_delete = false AND projects.archived = false LIMIT #{BATCH_SIZE} - SQL + SQL ) end - def bulk_insert_services(columns, values_array) - ActiveRecord::Base.connection.execute( - <<-SQL.strip_heredoc - INSERT INTO services (#{columns.join(', ')}) - VALUES #{values_array.map { |tuple| "(#{tuple.join(', ')})" }.join(', ')} - SQL - ) + def bulk_insert(klass, columns, values_array) + items_to_insert = values_array.map { |array| Hash[columns.zip(array)] } + + klass.insert_all(items_to_insert, returning: [:id]) end def service_hash - @service_hash ||= - begin - template_hash = @template.as_json(methods: :type).except('id', 'template', 'project_id') - - template_hash.each_with_object({}) do |(key, value), service_hash| - value = value.is_a?(Hash) ? value.to_json : value + @service_hash ||= template.as_json(methods: :type, except: %w[id template project_id]) + end - service_hash[ActiveRecord::Base.connection.quote_column_name(key)] = - ActiveRecord::Base.connection.quote(value) - end - end + def data_hash + @data_hash ||= template.data_fields.as_json(only: template.data_fields.class.column_names).except('id', 'service_id') end # rubocop: disable CodeReuse/ActiveRecord @@ -97,11 +97,11 @@ module Projects # rubocop: enable CodeReuse/ActiveRecord def active_external_issue_tracker? - @template.issue_tracker? && !@template.default + template.issue_tracker? && !template.default end def active_external_wiki? - @template.type == 'ExternalWikiService' + template.type == 'ExternalWikiService' end end end diff --git a/app/services/projects/update_repository_storage_service.rb b/app/services/projects/update_repository_storage_service.rb index 2e5de9411d1..65af112124e 100644 --- a/app/services/projects/update_repository_storage_service.rb +++ b/app/services/projects/update_repository_storage_service.rb @@ -18,7 +18,7 @@ module Projects mark_old_paths_for_archive - project.update(repository_storage: new_repository_storage_key, repository_read_only: false) + project.update!(repository_storage: new_repository_storage_key, repository_read_only: false) project.leave_pool_repository project.track_project_repository @@ -26,8 +26,8 @@ module Projects success - rescue Error, ArgumentError, Gitlab::Git::BaseError => e - project.update(repository_read_only: false) + rescue StandardError => e + project.update!(repository_read_only: false) Gitlab::ErrorTracking.track_exception(e, project_path: project.full_path) diff --git a/changelogs/unreleased/212549-padding-in-snippet-blob-header.yml b/changelogs/unreleased/212549-padding-in-snippet-blob-header.yml new file mode 100644 index 00000000000..faf1bf279bd --- /dev/null +++ b/changelogs/unreleased/212549-padding-in-snippet-blob-header.yml @@ -0,0 +1,5 @@ +--- +title: Fix minor spacing issue at Snippet blob viewer +merge_request: 29625 +author: Karthick Venkatesan +type: fixed diff --git a/changelogs/unreleased/214834-propagate-service-tempate.yml b/changelogs/unreleased/214834-propagate-service-tempate.yml new file mode 100644 index 00000000000..58b00cf6b72 --- /dev/null +++ b/changelogs/unreleased/214834-propagate-service-tempate.yml @@ -0,0 +1,5 @@ +--- +title: Propagation of service templates also covers services with separate data tables. +merge_request: 29805 +author: +type: fixed diff --git a/changelogs/unreleased/38096-add-index-on-resource-weight-events-created-at-pd.yml b/changelogs/unreleased/38096-add-index-on-resource-weight-events-created-at-pd.yml new file mode 100644 index 00000000000..ecaa3da4bc5 --- /dev/null +++ b/changelogs/unreleased/38096-add-index-on-resource-weight-events-created-at-pd.yml @@ -0,0 +1,5 @@ +--- +title: Add index to issue_id and created_at of resource_weight_events +merge_request: 28930 +author: +type: other diff --git a/changelogs/unreleased/shard_move_capture_all_errors.yml b/changelogs/unreleased/shard_move_capture_all_errors.yml new file mode 100644 index 00000000000..5c455672266 --- /dev/null +++ b/changelogs/unreleased/shard_move_capture_all_errors.yml @@ -0,0 +1,5 @@ +--- +title: Capture all errors when updating repository storage +merge_request: 30119 +author: +type: fixed diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index 417b5889bcf..721ca8ba475 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -13,8 +13,9 @@ MARKDOWN CATEGORY_TABLE_HEADER = <<MARKDOWN To spread load more evenly across eligible reviewers, Danger has randomly picked -a candidate for each review slot. Feel free to override this selection if you -think someone else would be better-suited, or the chosen person is unavailable. +a candidate for each review slot. Feel free to +[override these selections](https://about.gitlab.com/handbook/engineering/projects/#gitlab) +if you think someone else would be better-suited, or the chosen person is unavailable. To read more on how to use the reviewer roulette, please take a look at the [Engineering workflow](https://about.gitlab.com/handbook/engineering/workflow/#basics) diff --git a/db/migrate/20200406141452_add_index_to_issue_id_and_created_at_on_resource_weight_events.rb b/db/migrate/20200406141452_add_index_to_issue_id_and_created_at_on_resource_weight_events.rb new file mode 100644 index 00000000000..94a1b589ff9 --- /dev/null +++ b/db/migrate/20200406141452_add_index_to_issue_id_and_created_at_on_resource_weight_events.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddIndexToIssueIdAndCreatedAtOnResourceWeightEvents < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + INDEX_NAME = 'index_resource_weight_events_on_issue_id_and_created_at' + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :resource_weight_events, [:issue_id, :created_at], name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :resource_weight_events, INDEX_NAME + end +end diff --git a/db/migrate/20200415153154_add_unique_index_on_plan_name.rb b/db/migrate/20200415153154_add_unique_index_on_plan_name.rb new file mode 100644 index 00000000000..d959d1315b4 --- /dev/null +++ b/db/migrate/20200415153154_add_unique_index_on_plan_name.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddUniqueIndexOnPlanName < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + remove_concurrent_index :plans, :name + add_concurrent_index :plans, :name, unique: true + end + + def down + remove_concurrent_index :plans, :name, unique: true + add_concurrent_index :plans, :name + end +end diff --git a/db/structure.sql b/db/structure.sql index 9dc89aeeb1f..cc9a332be97 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9872,7 +9872,7 @@ CREATE INDEX index_personal_access_tokens_on_user_id ON public.personal_access_t CREATE UNIQUE INDEX index_plan_limits_on_plan_id ON public.plan_limits USING btree (plan_id); -CREATE INDEX index_plans_on_name ON public.plans USING btree (name); +CREATE UNIQUE INDEX index_plans_on_name ON public.plans USING btree (name); CREATE UNIQUE INDEX index_pool_repositories_on_disk_path ON public.pool_repositories USING btree (disk_path); @@ -10160,6 +10160,8 @@ CREATE INDEX index_resource_milestone_events_on_milestone_id ON public.resource_ CREATE INDEX index_resource_milestone_events_on_user_id ON public.resource_milestone_events USING btree (user_id); +CREATE INDEX index_resource_weight_events_on_issue_id_and_created_at ON public.resource_weight_events USING btree (issue_id, created_at); + CREATE INDEX index_resource_weight_events_on_issue_id_and_weight ON public.resource_weight_events USING btree (issue_id, weight); CREATE INDEX index_resource_weight_events_on_user_id ON public.resource_weight_events USING btree (user_id); @@ -13232,6 +13234,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200406102111 20200406102120 20200406135648 +20200406141452 20200406192059 20200406193427 20200407094005 @@ -13262,6 +13265,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200413072059 20200413230056 20200414144547 +20200415153154 20200415160722 20200415161021 20200415161206 diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 17de12381be..749461b5e9e 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -341,6 +341,11 @@ type BoardList { label: Label """ + The current limit metric for the list + """ + limitMetric: ListLimitMetric + + """ Type of the list """ listType: String! @@ -4620,6 +4625,15 @@ type LabelEdge { } """ +List limit metric setting +""" +enum ListLimitMetric { + all_metrics + issue_count + issue_weights +} + +""" Autogenerated input type of MarkAsSpamSnippet """ input MarkAsSpamSnippetInput { diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 0e4087794ba..70f9a53244e 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -1037,6 +1037,20 @@ "deprecationReason": null }, { + "name": "limitMetric", + "description": "The current limit metric for the list", + "args": [ + + ], + "type": { + "kind": "ENUM", + "name": "ListLimitMetric", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { "name": "listType", "description": "Type of the list", "args": [ @@ -13172,6 +13186,35 @@ "possibleTypes": null }, { + "kind": "ENUM", + "name": "ListLimitMetric", + "description": "List limit metric setting", + "fields": null, + "inputFields": null, + "interfaces": null, + "enumValues": [ + { + "name": "all_metrics", + "description": null, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "issue_count", + "description": null, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "issue_weights", + "description": null, + "isDeprecated": false, + "deprecationReason": null + } + ], + "possibleTypes": null + }, + { "kind": "INPUT_OBJECT", "name": "MarkAsSpamSnippetInput", "description": "Autogenerated input type of MarkAsSpamSnippet", diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 89cf4e9ab39..af126f48c50 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -89,6 +89,7 @@ Represents a list for an issue board | `collapsed` | Boolean | Indicates if list is collapsed for this user | | `id` | ID! | ID (global ID) of the list | | `label` | Label | Label of the list | +| `limitMetric` | ListLimitMetric | The current limit metric for the list | | `listType` | String! | Type of the list | | `maxIssueCount` | Int | Maximum number of issues in the list | | `maxIssueWeight` | Int | Maximum weight of issues in the list | diff --git a/doc/development/application_limits.md b/doc/development/application_limits.md index ace86e0ded2..b13e2994c52 100644 --- a/doc/development/application_limits.md +++ b/doc/development/application_limits.md @@ -90,7 +90,7 @@ project.actual_limits.exceeded?(:project_hooks, 10) #### `Limitable` concern -The [`Limitable` concern](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/app/models/concerns/limitable.rb) +The [`Limitable` concern](https://gitlab.com/gitlab-org/gitlab/blob/master/app/models/concerns/limitable.rb) can be used to validate that a model does not exceed the limits. It ensures that the count of the records for the current model does not exceed the defined limit. diff --git a/doc/install/aws/index.md b/doc/install/aws/index.md index c9ad462f289..ebd39354cb5 100644 --- a/doc/install/aws/index.md +++ b/doc/install/aws/index.md @@ -533,6 +533,22 @@ Since we're adding our SSL certificate at the load balancer, we do not need GitL sudo gitlab-ctl reconfigure ``` +#### Fast lookup of authorized SSH keys + +The public SSH keys for users allowed to access GitLab are stored in `/var/opt/gitlab/.ssh/authorized_keys`. Typically we'd use shared storage so that all the instances are able to access this file when a user performs a Git action over SSH. Since we do not have shared storage in our setup, we'll update our configuration to authorize SSH users via indexed lookup in the GitLab database. + +Follow the instructions at [Setting up fast lookup via GitLab Shell](../../administration/operations/fast_ssh_key_lookup.md#setting-up-fast-lookup-via-gitlab-shell) to switch from using the `authorized_keys` file to the database. + +If you do not configure fast lookup, Git actions over SSH will result in the following error: + +```shell +Permission denied (publickey). +fatal: Could not read from remote repository. + +Please make sure you have the correct access rights +and the repository exists. +``` + #### Configure host keys Ordinarily we would manually copy the contents (primary and public keys) of `/etc/ssh/` on the primary application server to `/etc/ssh` on all secondary servers. This prevents false man-in-the-middle-attack alerts when accessing servers in your High Availability cluster behind a load balancer. diff --git a/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb b/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb index 2cb6a76b6b3..7b4418191a3 100644 --- a/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb +++ b/qa/qa/specs/features/browser_ui/2_plan/issue/create_issue_spec.rb @@ -17,7 +17,7 @@ module QA end end - context 'when using attachments in comments', :object_storage, quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/issues/205408', type: :bug } do + context 'when using attachments in comments', :object_storage do let(:gif_file_name) { 'banana_sample.gif' } let(:file_to_attach) do File.absolute_path(File.join('spec', 'fixtures', gif_file_name)) diff --git a/spec/factories/plan_limits.rb b/spec/factories/plan_limits.rb new file mode 100644 index 00000000000..4aea09618d0 --- /dev/null +++ b/spec/factories/plan_limits.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :plan_limits do + plan + + trait :default_plan do + plan factory: :default_plan + end + end +end diff --git a/spec/factories/plans.rb b/spec/factories/plans.rb new file mode 100644 index 00000000000..81506edcf16 --- /dev/null +++ b/spec/factories/plans.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :plan do + Plan.all_plans.each do |plan| + factory :"#{plan}_plan" do + name { plan } + title { name.titleize } + initialize_with { Plan.find_or_create_by(name: plan) } + end + end + end +end diff --git a/spec/frontend/blob/components/__snapshots__/blob_header_spec.js.snap b/spec/frontend/blob/components/__snapshots__/blob_header_spec.js.snap index 2878ad492a4..7d868625956 100644 --- a/spec/frontend/blob/components/__snapshots__/blob_header_spec.js.snap +++ b/spec/frontend/blob/components/__snapshots__/blob_header_spec.js.snap @@ -9,7 +9,7 @@ exports[`Blob Header Default Actions rendering matches the snapshot 1`] = ` /> <div - class="file-actions d-none d-sm-block" + class="file-actions d-none d-sm-flex" > <viewer-switcher-stub value="simple" diff --git a/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js b/spec/frontend/ci_variable_list/ci_variable_list/ajax_variable_list_spec.js index a1377564073..93b185bd242 100644 --- a/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js +++ b/spec/frontend/ci_variable_list/ci_variable_list/ajax_variable_list_spec.js @@ -35,8 +35,8 @@ describe('AjaxFormVariableList', () => { maskableRegex: container.dataset.maskableRegex, }); - spyOn(ajaxVariableList, 'updateRowsWithPersistedVariables').and.callThrough(); - spyOn(ajaxVariableList.variableList, 'toggleEnableRow').and.callThrough(); + jest.spyOn(ajaxVariableList, 'updateRowsWithPersistedVariables'); + jest.spyOn(ajaxVariableList.variableList, 'toggleEnableRow'); }); afterEach(() => { @@ -44,7 +44,7 @@ describe('AjaxFormVariableList', () => { }); describe('onSaveClicked', () => { - it('shows loading spinner while waiting for the request', done => { + it('shows loading spinner while waiting for the request', () => { const loadingIcon = saveButton.querySelector('.js-ci-variables-save-loading-icon'); mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(() => { @@ -55,63 +55,47 @@ describe('AjaxFormVariableList', () => { expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true); - ajaxVariableList - .onSaveClicked() - .then(() => { - expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true); - }) - .then(done) - .catch(done.fail); + return ajaxVariableList.onSaveClicked().then(() => { + expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(true); + }); }); - it('calls `updateRowsWithPersistedVariables` with the persisted variables', done => { + it('calls `updateRowsWithPersistedVariables` with the persisted variables', () => { const variablesResponse = [{ id: 1, key: 'foo', value: 'bar' }]; mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200, { variables: variablesResponse, }); - ajaxVariableList - .onSaveClicked() - .then(() => { - expect(ajaxVariableList.updateRowsWithPersistedVariables).toHaveBeenCalledWith( - variablesResponse, - ); - }) - .then(done) - .catch(done.fail); + return ajaxVariableList.onSaveClicked().then(() => { + expect(ajaxVariableList.updateRowsWithPersistedVariables).toHaveBeenCalledWith( + variablesResponse, + ); + }); }); - it('hides any previous error box', done => { + it('hides any previous error box', () => { mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200); expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true); - ajaxVariableList - .onSaveClicked() - .then(() => { - expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true); - }) - .then(done) - .catch(done.fail); + return ajaxVariableList.onSaveClicked().then(() => { + expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true); + }); }); - it('disables remove buttons while waiting for the request', done => { + it('disables remove buttons while waiting for the request', () => { mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(() => { expect(ajaxVariableList.variableList.toggleEnableRow).toHaveBeenCalledWith(false); return [200, {}]; }); - ajaxVariableList - .onSaveClicked() - .then(() => { - expect(ajaxVariableList.variableList.toggleEnableRow).toHaveBeenCalledWith(true); - }) - .then(done) - .catch(done.fail); + return ajaxVariableList.onSaveClicked().then(() => { + expect(ajaxVariableList.variableList.toggleEnableRow).toHaveBeenCalledWith(true); + }); }); - it('hides secret values', done => { + it('hides secret values', () => { mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(200, {}); const row = container.querySelector('.js-row'); @@ -124,46 +108,34 @@ describe('AjaxFormVariableList', () => { expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(true); expect(valueInput.classList.contains(HIDE_CLASS)).toBe(false); - ajaxVariableList - .onSaveClicked() - .then(() => { - expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(false); - expect(valueInput.classList.contains(HIDE_CLASS)).toBe(true); - }) - .then(done) - .catch(done.fail); + return ajaxVariableList.onSaveClicked().then(() => { + expect(valuePlaceholder.classList.contains(HIDE_CLASS)).toBe(false); + expect(valueInput.classList.contains(HIDE_CLASS)).toBe(true); + }); }); - it('shows error box with validation errors', done => { + it('shows error box with validation errors', () => { const validationError = 'some validation error'; mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(400, [validationError]); expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true); - ajaxVariableList - .onSaveClicked() - .then(() => { - expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(false); - expect(errorBox.textContent.trim().replace(/\n+\s+/m, ' ')).toEqual( - `Validation failed ${validationError}`, - ); - }) - .then(done) - .catch(done.fail); + return ajaxVariableList.onSaveClicked().then(() => { + expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(false); + expect(errorBox.textContent.trim().replace(/\n+\s+/m, ' ')).toEqual( + `Validation failed ${validationError}`, + ); + }); }); - it('shows flash message when request fails', done => { + it('shows flash message when request fails', () => { mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(500); expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true); - ajaxVariableList - .onSaveClicked() - .then(() => { - expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true); - }) - .then(done) - .catch(done.fail); + return ajaxVariableList.onSaveClicked().then(() => { + expect(errorBox.classList.contains(HIDE_CLASS)).toEqual(true); + }); }); }); diff --git a/spec/javascripts/ci_variable_list/ci_variable_list_spec.js b/spec/frontend/ci_variable_list/ci_variable_list/ci_variable_list_spec.js index c0c3a83a44b..9508203e5c2 100644 --- a/spec/javascripts/ci_variable_list/ci_variable_list_spec.js +++ b/spec/frontend/ci_variable_list/ci_variable_list/ci_variable_list_spec.js @@ -1,5 +1,5 @@ import $ from 'jquery'; -import getSetTimeoutPromise from 'spec/helpers/set_timeout_promise_helper'; +import waitForPromises from 'helpers/wait_for_promises'; import VariableList from '~/ci_variable_list/ci_variable_list'; const HIDE_CLASS = 'hide'; @@ -127,86 +127,74 @@ describe('VariableList', () => { variableList.init(); }); - it('should not add another row when editing the last rows protected checkbox', done => { + it('should not add another row when editing the last rows protected checkbox', () => { const $row = $wrapper.find('.js-row:last-child'); $row.find('.ci-variable-protected-item .js-project-feature-toggle').click(); - getSetTimeoutPromise() - .then(() => { - expect($wrapper.find('.js-row').length).toBe(1); - }) - .then(done) - .catch(done.fail); + return waitForPromises().then(() => { + expect($wrapper.find('.js-row').length).toBe(1); + }); }); - it('should not add another row when editing the last rows masked checkbox', done => { + it('should not add another row when editing the last rows masked checkbox', () => { + jest.spyOn(variableList, 'checkIfRowTouched'); const $row = $wrapper.find('.js-row:last-child'); $row.find('.ci-variable-masked-item .js-project-feature-toggle').click(); - getSetTimeoutPromise() - .then(() => { - expect($wrapper.find('.js-row').length).toBe(1); - }) - .then(done) - .catch(done.fail); + return waitForPromises().then(() => { + // This validates that we are checking after the event listener has run + expect(variableList.checkIfRowTouched).toHaveBeenCalled(); + expect($wrapper.find('.js-row').length).toBe(1); + }); }); describe('validateMaskability', () => { let $row; const maskingErrorElement = '.js-row:last-child .masking-validation-error'; + const clickToggle = () => + $row.find('.ci-variable-masked-item .js-project-feature-toggle').click(); beforeEach(() => { $row = $wrapper.find('.js-row:last-child'); - $row.find('.ci-variable-masked-item .js-project-feature-toggle').click(); }); it('has a regex provided via a data attribute', () => { + clickToggle(); + expect($wrapper.attr('data-maskable-regex')).toBe('^[a-zA-Z0-9_+=/@:.-]{8,}$'); }); - it('allows values that are 8 characters long', done => { + it('allows values that are 8 characters long', () => { $row.find('.js-ci-variable-input-value').val('looooong'); - getSetTimeoutPromise() - .then(() => { - expect($wrapper.find(maskingErrorElement)).toHaveClass('hide'); - }) - .then(done) - .catch(done.fail); + clickToggle(); + + expect($wrapper.find(maskingErrorElement)).toHaveClass('hide'); }); - it('rejects values that are shorter than 8 characters', done => { + it('rejects values that are shorter than 8 characters', () => { $row.find('.js-ci-variable-input-value').val('short'); - getSetTimeoutPromise() - .then(() => { - expect($wrapper.find(maskingErrorElement)).toBeVisible(); - }) - .then(done) - .catch(done.fail); + clickToggle(); + + expect($wrapper.find(maskingErrorElement)).toBeVisible(); }); - it('allows values with base 64 characters', done => { + it('allows values with base 64 characters', () => { $row.find('.js-ci-variable-input-value').val('abcABC123_+=/-'); - getSetTimeoutPromise() - .then(() => { - expect($wrapper.find(maskingErrorElement)).toHaveClass('hide'); - }) - .then(done) - .catch(done.fail); + clickToggle(); + + expect($wrapper.find(maskingErrorElement)).toHaveClass('hide'); }); - it('rejects values with other special characters', done => { + it('rejects values with other special characters', () => { $row.find('.js-ci-variable-input-value').val('1234567$'); - getSetTimeoutPromise() - .then(() => { - expect($wrapper.find(maskingErrorElement)).toBeVisible(); - }) - .then(done) - .catch(done.fail); + clickToggle(); + + expect($wrapper.find(maskingErrorElement)).toBeVisible(); }); }); }); diff --git a/spec/javascripts/ci_variable_list/native_form_variable_list_spec.js b/spec/frontend/ci_variable_list/ci_variable_list/native_form_variable_list_spec.js index 4982b68fa81..4982b68fa81 100644 --- a/spec/javascripts/ci_variable_list/native_form_variable_list_spec.js +++ b/spec/frontend/ci_variable_list/ci_variable_list/native_form_variable_list_spec.js diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index 4ed4b7e38d8..60615a7458b 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -17,6 +17,10 @@ describe Ci::PipelineSchedule do it { is_expected.to respond_to(:description) } it { is_expected.to respond_to(:next_run_at) } + it_behaves_like 'includes Limitable concern' do + subject { build(:ci_pipeline_schedule) } + end + describe 'validations' do it 'does not allow invalid cron patters' do pipeline_schedule = build(:ci_pipeline_schedule, cron: '0 0 0 * *') diff --git a/spec/models/hooks/project_hook_spec.rb b/spec/models/hooks/project_hook_spec.rb index a945f0d1516..ccf8171049d 100644 --- a/spec/models/hooks/project_hook_spec.rb +++ b/spec/models/hooks/project_hook_spec.rb @@ -11,6 +11,10 @@ describe ProjectHook do it { is_expected.to validate_presence_of(:project) } end + it_behaves_like 'includes Limitable concern' do + subject { build(:project_hook, project: create(:project)) } + end + describe '.push_hooks' do it 'returns hooks for push events only' do hook = create(:project_hook, push_events: true) diff --git a/spec/models/plan_spec.rb b/spec/models/plan_spec.rb new file mode 100644 index 00000000000..3f3b8046232 --- /dev/null +++ b/spec/models/plan_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Plan do + describe '#default?' do + subject { plan.default? } + + Plan.default_plans.each do |plan| + context "when '#{plan}'" do + let(:plan) { build("#{plan}_plan".to_sym) } + + it { is_expected.to be_truthy } + end + end + end +end diff --git a/spec/requests/api/graphql/boards/board_lists_query_spec.rb b/spec/requests/api/graphql/boards/board_lists_query_spec.rb index 711c20784b0..3d2f6cc9046 100644 --- a/spec/requests/api/graphql/boards/board_lists_query_spec.rb +++ b/spec/requests/api/graphql/boards/board_lists_query_spec.rb @@ -27,23 +27,23 @@ describe 'get board lists' do board_parent_type, { 'fullPath' => board_parent.full_path }, <<~BOARDS - boards(first: 1) { - edges { - node { - #{field_with_params('lists', list_params)} { - pageInfo { - startCursor - endCursor - } - edges { - node { - #{all_graphql_fields_for('board_lists'.classify)} + boards(first: 1) { + edges { + node { + #{field_with_params('lists', list_params)} { + pageInfo { + startCursor + endCursor + } + edges { + node { + #{all_graphql_fields_for('board_lists'.classify)} + } } } } } } - } BOARDS ) end diff --git a/spec/services/projects/propagate_service_template_spec.rb b/spec/services/projects/propagate_service_template_spec.rb index 2c3effec617..7188ac5f733 100644 --- a/spec/services/projects/propagate_service_template_spec.rb +++ b/spec/services/projects/propagate_service_template_spec.rb @@ -8,16 +8,19 @@ describe Projects::PropagateServiceTemplate do PushoverService.create( template: true, active: true, + push_events: false, properties: { device: 'MyDevice', sound: 'mic', priority: 4, user_key: 'asdf', api_key: '123456789' - }) + } + ) end let!(:project) { create(:project) } + let(:excluded_attributes) { %w[id project_id template created_at updated_at title description] } it 'creates services for projects' do expect(project.pushover_service).to be_nil @@ -35,7 +38,7 @@ describe Projects::PropagateServiceTemplate do properties: { bamboo_url: 'http://gitlab.com', username: 'mic', - password: "password", + password: 'password', build_key: 'build' } ) @@ -54,7 +57,7 @@ describe Projects::PropagateServiceTemplate do properties: { bamboo_url: 'http://gitlab.com', username: 'mic', - password: "password", + password: 'password', build_key: 'build' } ) @@ -70,6 +73,33 @@ describe Projects::PropagateServiceTemplate do described_class.propagate(service_template) expect(project.pushover_service.properties).to eq(service_template.properties) + + expect(project.pushover_service.attributes.except(*excluded_attributes)) + .to eq(service_template.attributes.except(*excluded_attributes)) + end + + context 'service with data fields' do + let(:service_template) do + JiraService.create!( + template: true, + active: true, + push_events: false, + url: 'http://jira.instance.com', + username: 'user', + password: 'secret' + ) + end + + it 'creates the service containing the template attributes' do + described_class.propagate(service_template) + + expect(project.jira_service.attributes.except(*excluded_attributes)) + .to eq(service_template.attributes.except(*excluded_attributes)) + + excluded_attributes = %w[id service_id created_at updated_at] + expect(project.jira_service.data_fields.attributes.except(*excluded_attributes)) + .to eq(service_template.data_fields.attributes.except(*excluded_attributes)) + end end describe 'bulk update', :use_sql_query_cache do diff --git a/spec/services/projects/update_repository_storage_service_spec.rb b/spec/services/projects/update_repository_storage_service_spec.rb index 05555fa76f7..97df388fc15 100644 --- a/spec/services/projects/update_repository_storage_service_spec.rb +++ b/spec/services/projects/update_repository_storage_service_spec.rb @@ -12,6 +12,7 @@ describe Projects::UpdateRepositoryStorageService do before do allow(Time).to receive(:now).and_return(time) + allow(Gitlab.config.repositories.storages).to receive(:keys).and_return(%w[default test_second_storage]) end context 'without wiki and design repository' do diff --git a/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb b/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb new file mode 100644 index 00000000000..4bcea36fd42 --- /dev/null +++ b/spec/support/shared_examples/models/concerns/limitable_shared_examples.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'includes Limitable concern' do + describe 'validations' do + let(:plan_limits) { create(:plan_limits, :default_plan) } + + it { is_expected.to be_a(Limitable) } + + context 'without plan limits configured' do + it 'can create new models' do + expect { subject.save }.to change { described_class.count } + end + end + + context 'with plan limits configured' do + before do + plan_limits.update(subject.class.limit_name => 1) + end + + it 'can create new models' do + expect { subject.save }.to change { described_class.count } + end + + context 'with an existing model' do + before do + subject.dup.save + end + + it 'cannot create new models exceding the plan limits' do + expect { subject.save }.not_to change { described_class.count } + expect(subject.errors[:base]).to contain_exactly("Maximum number of #{subject.class.limit_name.humanize(capitalize: false)} (1) exceeded") + end + end + end + end +end diff --git a/spec/support/shared_examples/workers/pages_domain_cron_worker_shared_examples.rb b/spec/support/shared_examples/workers/pages_domain_cron_worker_shared_examples.rb index 9e8102aea53..c79e3ed7d21 100644 --- a/spec/support/shared_examples/workers/pages_domain_cron_worker_shared_examples.rb +++ b/spec/support/shared_examples/workers/pages_domain_cron_worker_shared_examples.rb @@ -3,12 +3,14 @@ RSpec.shared_examples 'a pages cronjob scheduling jobs with context' do |scheduled_worker_class| let(:worker) { described_class.new } - it 'does not cause extra queries for multiple domains' do - control = ActiveRecord::QueryRecorder.new { worker.perform } + context 'with RequestStore enabled', :request_store do + it 'does not cause extra queries for multiple domains' do + control = ActiveRecord::QueryRecorder.new { worker.perform } - extra_domain + extra_domain - expect { worker.perform }.not_to exceed_query_limit(control) + expect { worker.perform }.not_to exceed_query_limit(control) + end end it 'schedules the renewal with a context' do |