diff --git a/app/assets/javascripts/pager.js b/app/assets/javascripts/pager.js index aa2f539b6e2..e15766a7839 100644 --- a/app/assets/javascripts/pager.js +++ b/app/assets/javascripts/pager.js @@ -22,7 +22,10 @@ export default { this.prepareData = prepareData; this.successCallback = successCallback; this.errorCallback = errorCallback; - this.loading = $(`${container} .loading`).first(); + this.$container = $(container); + this.$loading = this.$container.length + ? this.$container.find('.loading').first() + : $('.loading').first(); if (preload) { this.offset = 0; this.getOld(); @@ -31,7 +34,7 @@ export default { }, getOld() { - this.loading.show(); + this.$loading.show(); const url = $('.content_list').data('href') || removeParams(['limit', 'offset']); axios @@ -49,11 +52,11 @@ export default { if (!this.disable && !this.isScrollable()) { this.getOld(); } else { - this.loading.hide(); + this.$loading.hide(); } }) .catch((err) => this.errorCallback(err)) - .finally(() => this.loading.hide()); + .finally(() => this.$loading.hide()); }, append(count, html) { @@ -83,8 +86,12 @@ export default { fireOnce: true, ceaseFire: () => this.disable === true, callback: () => { - if (!this.loading.is(':visible')) { - this.loading.show(); + if (this.$container.length && !this.$container.is(':visible')) { + return; + } + + if (!this.$loading.is(':visible')) { + this.$loading.show(); this.getOld(); } }, diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index a8528cfbcff..21c7a54670c 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -650,14 +650,6 @@ module ProjectsHelper "You must enable HTTPS for all your domains first" end - def pages_https_only_label_class - if pages_https_only_disabled? - "list-label disabled" - else - "list-label" - end - end - def filter_starrer_path(options = {}) options = params.slice(:sort).merge(options).permit! "#{request.path}?#{options.to_param}" diff --git a/app/models/clusters/agent_token.rb b/app/models/clusters/agent_token.rb index 691d628524f..1607d0b6d19 100644 --- a/app/models/clusters/agent_token.rb +++ b/app/models/clusters/agent_token.rb @@ -18,7 +18,7 @@ module Clusters validates :description, length: { maximum: 1024 } validates :name, presence: true, length: { maximum: 255 } - scope :order_last_used_at_desc, -> { order(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) } + scope :order_last_used_at_desc, -> { order(arel_table[:last_used_at].desc.nulls_last) } scope :with_status, -> (status) { where(status: status) } enum status: { diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index f4763fdf14b..dbd760a9c45 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -318,12 +318,16 @@ module Issuable # 2. We can't ORDER BY a column that isn't in the GROUP BY and doesn't # have an aggregate function applied, so we do a useless MIN() instead. # - milestones_due_date = 'MIN(milestones.due_date)' + milestones_due_date = Milestone.arel_table[:due_date].minimum + milestones_due_date_with_direction = direction == 'ASC' ? milestones_due_date.asc : milestones_due_date.desc + + highest_priority_arel = Arel.sql('highest_priority') + highest_priority_arel_with_direction = direction == 'ASC' ? highest_priority_arel.asc : highest_priority_arel.desc order_milestone_due_asc .order_labels_priority(excluded_labels: excluded_labels, extra_select_columns: [milestones_due_date]) - .reorder(Gitlab::Database.nulls_last_order(milestones_due_date, direction), - Gitlab::Database.nulls_last_order('highest_priority', direction)) + .reorder(milestones_due_date_with_direction.nulls_last, + highest_priority_arel_with_direction.nulls_last) end def order_labels_priority(direction = 'ASC', excluded_labels: [], extra_select_columns: [], with_cte: false) @@ -341,12 +345,15 @@ module Issuable extra_select_columns.unshift("highest_priorities.label_priority as highest_priority") + highest_priority_arel = Arel.sql('highest_priority') + highest_priority_arel_with_direction = direction == 'ASC' ? highest_priority_arel.asc : highest_priority_arel.desc + select(issuable_columns) .select(extra_select_columns) .from("#{table_name}") .joins("JOIN LATERAL(#{highest_priority}) as highest_priorities ON TRUE") .group(group_columns) - .reorder(Gitlab::Database.nulls_last_order('highest_priority', direction)) + .reorder(highest_priority_arel_with_direction.nulls_last) end def with_label(title, sort = nil) diff --git a/app/models/deploy_token.rb b/app/models/deploy_token.rb index 362a86aebd0..360a9ffbc53 100644 --- a/app/models/deploy_token.rb +++ b/app/models/deploy_token.rb @@ -14,6 +14,9 @@ class DeployToken < ApplicationRecord default_value_for(:expires_at) { Forever.date } + # Do NOT use this `user` for the authentication/authorization of the deploy tokens. + # It's for the auditing purpose on Credential Inventory, only. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/353467#note_859774246 for more information. belongs_to :user, foreign_key: :creator_id, optional: true has_many :project_deploy_tokens, inverse_of: :deploy_token diff --git a/app/models/environment.rb b/app/models/environment.rb index 4c4ba22fae1..9e663b2ee74 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -66,10 +66,10 @@ class Environment < ApplicationRecord scope :stopped, -> { with_state(:stopped) } scope :order_by_last_deployed_at, -> do - order(Gitlab::Database.nulls_first_order("(#{max_deployment_id_sql})", 'ASC')) + order(Arel::Nodes::Grouping.new(max_deployment_id_query).asc.nulls_first) end scope :order_by_last_deployed_at_desc, -> do - order(Gitlab::Database.nulls_last_order("(#{max_deployment_id_sql})", 'DESC')) + order(Arel::Nodes::Grouping.new(max_deployment_id_query).desc.nulls_last) end scope :order_by_name, -> { order('environments.name ASC') } @@ -151,10 +151,11 @@ class Environment < ApplicationRecord find_by(id: id, slug: slug) end - def self.max_deployment_id_sql - Deployment.select(Deployment.arel_table[:id].maximum) - .where(Deployment.arel_table[:environment_id].eq(arel_table[:id])) - .to_sql + def self.max_deployment_id_query + Arel.sql( + Deployment.select(Deployment.arel_table[:id].maximum) + .where(Deployment.arel_table[:environment_id].eq(arel_table[:id])).to_sql + ) end def self.pluck_names diff --git a/app/models/issue.rb b/app/models/issue.rb index 7bb388ed4dc..c2b8b457049 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -118,15 +118,15 @@ class Issue < ApplicationRecord scope :not_authored_by, ->(user) { where.not(author_id: user) } - scope :order_due_date_asc, -> { reorder(::Gitlab::Database.nulls_last_order('due_date', 'ASC')) } - scope :order_due_date_desc, -> { reorder(::Gitlab::Database.nulls_last_order('due_date', 'DESC')) } + scope :order_due_date_asc, -> { reorder(arel_table[:due_date].asc.nulls_last) } + scope :order_due_date_desc, -> { reorder(arel_table[:due_date].desc.nulls_last) } scope :order_closest_future_date, -> { reorder(Arel.sql('CASE WHEN issues.due_date >= CURRENT_DATE THEN 0 ELSE 1 END ASC, ABS(CURRENT_DATE - issues.due_date) ASC')) } scope :order_closed_date_desc, -> { reorder(closed_at: :desc) } scope :order_created_at_desc, -> { reorder(created_at: :desc) } scope :order_severity_asc, -> { includes(:issuable_severity).order('issuable_severities.severity ASC NULLS FIRST') } scope :order_severity_desc, -> { includes(:issuable_severity).order('issuable_severities.severity DESC NULLS LAST') } - scope :order_escalation_status_asc, -> { includes(:incident_management_issuable_escalation_status).order(::Gitlab::Database.nulls_last_order('incident_management_issuable_escalation_status.status')) } - scope :order_escalation_status_desc, -> { includes(:incident_management_issuable_escalation_status).order(::Gitlab::Database.nulls_last_order('incident_management_issuable_escalation_status.status', 'DESC')) } + scope :order_escalation_status_asc, -> { includes(:incident_management_issuable_escalation_status).order(IncidentManagement::IssuableEscalationStatus.arel_table[:status].asc.nulls_last).references(:incident_management_issuable_escalation_status) } + scope :order_escalation_status_desc, -> { includes(:incident_management_issuable_escalation_status).order(IncidentManagement::IssuableEscalationStatus.arel_table[:status].desc.nulls_last).references(:incident_management_issuable_escalation_status) } scope :preload_associated_models, -> { preload(:assignees, :labels, project: :namespace) } scope :with_web_entity_associations, -> { preload(:author, project: [:project_feature, :route, namespace: :route]) } @@ -344,8 +344,8 @@ class Issue < ApplicationRecord Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'relative_position', column_expression: arel_table[:relative_position], - order_expression: Gitlab::Database.nulls_last_order('issues.relative_position', 'ASC'), - reversed_order_expression: Gitlab::Database.nulls_last_order('issues.relative_position', 'DESC'), + order_expression: Issue.arel_table[:relative_position].asc.nulls_last, + reversed_order_expression: Issue.arel_table[:relative_position].desc.nulls_last, order_direction: :asc, nullable: :nulls_last, distinct: false diff --git a/app/models/key.rb b/app/models/key.rb index ee51e082e3a..42ea0f29171 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -49,7 +49,7 @@ class Key < ApplicationRecord scope :preload_users, -> { preload(:user) } scope :for_user, -> (user) { where(user: user) } - scope :order_last_used_at_desc, -> { reorder(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) } + scope :order_last_used_at_desc, -> { reorder(arel_table[:last_used_at].desc.nulls_last) } # Date is set specifically in this scope to improve query time. scope :expired_today_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') = CURRENT_DATE AND expiry_notification_delivered_at IS NULL"]) } diff --git a/app/models/member.rb b/app/models/member.rb index 77e3a758605..18ad2785d6e 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -178,14 +178,14 @@ class Member < ApplicationRecord unscoped.from(distinct_members, :members) end - scope :order_name_asc, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.name', 'ASC')) } - scope :order_name_desc, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.name', 'DESC')) } - scope :order_recent_sign_in, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.last_sign_in_at', 'DESC')) } - scope :order_oldest_sign_in, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.last_sign_in_at', 'ASC')) } - scope :order_recent_last_activity, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.last_activity_on', 'DESC')) } - scope :order_oldest_last_activity, -> { left_join_users.reorder(Gitlab::Database.nulls_first_order('users.last_activity_on', 'ASC')) } - scope :order_recent_created_user, -> { left_join_users.reorder(Gitlab::Database.nulls_last_order('users.created_at', 'DESC')) } - scope :order_oldest_created_user, -> { left_join_users.reorder(Gitlab::Database.nulls_first_order('users.created_at', 'ASC')) } + scope :order_name_asc, -> { left_join_users.reorder(User.arel_table[:name].asc.nulls_last) } + scope :order_name_desc, -> { left_join_users.reorder(User.arel_table[:name].desc.nulls_last) } + scope :order_recent_sign_in, -> { left_join_users.reorder(User.arel_table[:last_sign_in_at].desc.nulls_last) } + scope :order_oldest_sign_in, -> { left_join_users.reorder(User.arel_table[:last_sign_in_at].asc.nulls_last) } + scope :order_recent_last_activity, -> { left_join_users.reorder(User.arel_table[:last_activity_on].desc.nulls_last) } + scope :order_oldest_last_activity, -> { left_join_users.reorder(User.arel_table[:last_activity_on].asc.nulls_first) } + scope :order_recent_created_user, -> { left_join_users.reorder(User.arel_table[:created_at].desc.nulls_last) } + scope :order_oldest_created_user, -> { left_join_users.reorder(User.arel_table[:created_at].asc.nulls_first) } scope :on_project_and_ancestors, ->(project) { where(source: [project] + project.ancestors) } diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 512fa294128..4c6ed399bf9 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -329,15 +329,15 @@ class MergeRequest < ApplicationRecord end scope :by_target_branch, ->(branch_name) { where(target_branch: branch_name) } scope :order_by_metric, ->(metric, direction) do - reverse_direction = { 'ASC' => 'DESC', 'DESC' => 'ASC' } - reversed_direction = reverse_direction[direction] || raise("Unknown sort direction was given: #{direction}") + column_expression = MergeRequest::Metrics.arel_table[metric] + column_expression_with_direction = direction == 'ASC' ? column_expression.asc : column_expression.desc order = Gitlab::Pagination::Keyset::Order.build([ Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: "merge_request_metrics_#{metric}", - column_expression: MergeRequest::Metrics.arel_table[metric], - order_expression: Gitlab::Database.nulls_last_order("merge_request_metrics.#{metric}", direction), - reversed_order_expression: Gitlab::Database.nulls_first_order("merge_request_metrics.#{metric}", reversed_direction), + column_expression: column_expression, + order_expression: column_expression_with_direction.nulls_last, + reversed_order_expression: column_expression_with_direction.reverse.nulls_first, order_direction: direction, nullable: :nulls_last, distinct: false, diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 86da29dd27a..ff4fadb0f13 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -31,7 +31,7 @@ class Milestone < ApplicationRecord end scope :order_by_name_asc, -> { order(Arel::Nodes::Ascending.new(arel_table[:title].lower)) } - scope :reorder_by_due_date_asc, -> { reorder(Gitlab::Database.nulls_last_order('due_date', 'ASC')) } + scope :reorder_by_due_date_asc, -> { reorder(arel_table[:due_date].asc.nulls_last) } scope :with_api_entity_associations, -> { preload(project: [:project_feature, :route, namespace: :route]) } scope :order_by_dates_and_title, -> { order(due_date: :asc, start_date: :asc, title: :asc) } @@ -116,15 +116,15 @@ class Milestone < ApplicationRecord when 'due_date_asc' reorder_by_due_date_asc when 'due_date_desc' - reorder(Gitlab::Database.nulls_last_order('due_date', 'DESC')) + reorder(arel_table[:due_date].desc.nulls_last) when 'name_asc' reorder(Arel::Nodes::Ascending.new(arel_table[:title].lower)) when 'name_desc' reorder(Arel::Nodes::Descending.new(arel_table[:title].lower)) when 'start_date_asc' - reorder(Gitlab::Database.nulls_last_order('start_date', 'ASC')) + reorder(arel_table[:start_date].asc.nulls_last) when 'start_date_desc' - reorder(Gitlab::Database.nulls_last_order('start_date', 'DESC')) + reorder(arel_table[:start_date].desc.nulls_last) else order_by(method) end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 5dcf240b6c3..3b75b6d163a 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -132,7 +132,7 @@ class Namespace < ApplicationRecord scope :user_namespaces, -> { where(type: Namespaces::UserNamespace.sti_name) } scope :without_project_namespaces, -> { where(Namespace.arel_table[:type].not_eq(Namespaces::ProjectNamespace.sti_name)) } - scope :sort_by_type, -> { order(Gitlab::Database.nulls_first_order(:type)) } + scope :sort_by_type, -> { order(arel_table[:type].asc.nulls_first) } scope :include_route, -> { includes(:route) } scope :by_parent, -> (parent) { where(parent_id: parent) } scope :filter_by_path, -> (query) { where('lower(path) = :query', query: query.downcase) } diff --git a/app/models/packages/package.rb b/app/models/packages/package.rb index c76473c9438..7744e578df5 100644 --- a/app/models/packages/package.rb +++ b/app/models/packages/package.rb @@ -228,8 +228,8 @@ class Packages::Package < ApplicationRecord def self.keyset_pagination_order(join_class:, column_name:, direction: :asc) join_table = join_class.table_name - asc_order_expression = Gitlab::Database.nulls_last_order("#{join_table}.#{column_name}", :asc) - desc_order_expression = Gitlab::Database.nulls_first_order("#{join_table}.#{column_name}", :desc) + asc_order_expression = join_class.arel_table[column_name].asc.nulls_last + desc_order_expression = join_class.arel_table[column_name].desc.nulls_first order_direction = direction == :asc ? asc_order_expression : desc_order_expression reverse_order_direction = direction == :asc ? desc_order_expression : asc_order_expression arel_order_classes = ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::AREL_ORDER_CLASSES.invert diff --git a/app/models/project.rb b/app/models/project.rb index 6e5c694cb9c..f7182d1645c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2876,6 +2876,11 @@ class Project < ApplicationRecord Projects::RecordTargetPlatformsWorker.perform_async(id) end + def inactive? + (statistics || build_statistics).storage_size > ::Gitlab::CurrentSettings.inactive_projects_min_size_mb.megabytes && + last_activity_at < ::Gitlab::CurrentSettings.inactive_projects_send_warning_email_after_months.months.ago + end + private # overridden in EE diff --git a/app/models/todo.rb b/app/models/todo.rb index eb5d9965955..45ab770a0f6 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -148,10 +148,10 @@ class Todo < ApplicationRecord target_type_column: "todos.target_type", target_column: "todos.target_id", project_column: "todos.project_id" - ).to_sql + ).arel.as('highest_priority') - select("#{table_name}.*, (#{highest_priority}) AS highest_priority") - .order(Gitlab::Database.nulls_last_order('highest_priority', 'ASC')) + select(arel_table[Arel.star], highest_priority) + .order(Arel.sql('highest_priority').asc.nulls_last) .order('todos.created_at') end diff --git a/app/models/user.rb b/app/models/user.rb index d97ada1ecec..26d47de4f00 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -470,10 +470,10 @@ class User < ApplicationRecord .where('keys.user_id = users.id') .expiring_soon_and_not_notified) end - scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) } - scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) } - scope :order_recent_last_activity, -> { reorder(Gitlab::Database.nulls_last_order('last_activity_on', 'DESC')) } - scope :order_oldest_last_activity, -> { reorder(Gitlab::Database.nulls_first_order('last_activity_on', 'ASC')) } + scope :order_recent_sign_in, -> { reorder(arel_table[:current_sign_in_at].desc.nulls_last) } + scope :order_oldest_sign_in, -> { reorder(arel_table[:current_sign_in_at].asc.nulls_last) } + scope :order_recent_last_activity, -> { reorder(arel_table[:last_activity_on].desc.nulls_last) } + scope :order_oldest_last_activity, -> { reorder(arel_table[:last_activity_on].asc.nulls_first) } scope :by_id_and_login, ->(id, login) { where(id: id).where('username = LOWER(:login) OR email = LOWER(:login)', login: login) } scope :dormant, -> { with_state(:active).human_or_service_user.where('last_activity_on <= ?', MINIMUM_INACTIVE_DAYS.day.ago.to_date) } scope :with_no_activity, -> { with_state(:active).human_or_service_user.where(last_activity_on: nil) } diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index 28ea1ac8296..f7ffe288d57 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -75,7 +75,6 @@ module ResourceAccessTokens end def generate_email - # Default emaildomain need to be reworked. See gitlab-org/gitlab#260305 email_pattern = "#{resource_type}#{resource.id}_bot%s@noreply.#{Gitlab.config.gitlab.host}" uniquify.string(-> (n) { Kernel.sprintf(email_pattern, n) }) do |s| diff --git a/app/views/admin/application_settings/_localization.html.haml b/app/views/admin/application_settings/_localization.html.haml index d0bb6a78ed6..a6ed48ef4fe 100644 --- a/app/views/admin/application_settings/_localization.html.haml +++ b/app/views/admin/application_settings/_localization.html.haml @@ -1,4 +1,4 @@ -= form_for @application_setting, url: preferences_admin_application_settings_path(anchor: 'js-localization-settings'), html: { class: 'fieldset-form' } do |f| += gitlab_ui_form_for @application_setting, url: preferences_admin_application_settings_path(anchor: 'js-localization-settings'), html: { class: 'fieldset-form' } do |f| = form_errors(@application_setting) %fieldset @@ -11,13 +11,9 @@ .form-group = f.label :time_tracking, _('Time tracking'), class: 'label-bold' - .form-check - = f.check_box :time_tracking_limit_to_hours, class: 'form-check-input' - = f.label :time_tracking_limit_to_hours, class: 'form-check-label' do - = _('Limit display of time tracking units to hours.') - .form-text.text-muted - = _('Display time tracking in issues in total hours only.') - = link_to _('What is time tracking?'), help_page_path('user/project/time_tracking.md'), target: '_blank', rel: 'noopener noreferrer' + - time_tracking_help_link = help_page_path('user/project/time_tracking.md') + - time_tracking_help_link_start = ''.html_safe % { url: time_tracking_help_link } + = f.gitlab_ui_checkbox_component :time_tracking_limit_to_hours, _('Limit display of time tracking units to hours.'), help_text: _('Display time tracking in issues in total hours only. %{link_start}What is time tracking?%{link_end}').html_safe % { link_start: time_tracking_help_link_start, link_end: ''.html_safe } = f.submit _('Save changes'), class: "gl-button btn btn-confirm" diff --git a/app/views/projects/pages/_pages_settings.html.haml b/app/views/projects/pages/_pages_settings.html.haml index 15fb5755b61..0010564081e 100644 --- a/app/views/projects/pages/_pages_settings.html.haml +++ b/app/views/projects/pages/_pages_settings.html.haml @@ -2,21 +2,20 @@ - can_enforce_https_only=Gitlab.config.pages.external_http || Gitlab.config.pages.external_https - return unless can_edit_max_page_size || can_enforce_https_only -= form_for @project, url: project_pages_path(@project), html: { class: 'inline', title: pages_https_only_title } do |f| += gitlab_ui_form_for @project, url: project_pages_path(@project), html: { class: 'inline', title: pages_https_only_title } do |f| - if can_edit_max_page_size = render_if_exists 'shared/pages/max_pages_size_input', form: f - if can_enforce_https_only .form-group - .form-check - = f.check_box :pages_https_only, class: 'form-check-input', disabled: pages_https_only_disabled? - = f.label :pages_https_only, class: pages_https_only_label_class do - %strong - = s_('GitLabPages|Force HTTPS (requires valid certificates)') - - docs_link_start = "".html_safe - - link_end = ''.html_safe - %p - = s_("GitLabPages|When enabled, all attempts to visit your website through HTTP are automatically redirected to HTTPS using a response with status code 301. Requires a valid certificate for all domains. %{docs_link_start}Learn more.%{link_end}").html_safe % { docs_link_start: docs_link_start, link_end: link_end } + = f.gitlab_ui_checkbox_component :pages_https_only, + s_('GitLabPages|Force HTTPS (requires valid certificates)'), + checkbox_options: { disabled: pages_https_only_disabled? }, + label_options: { class: 'label-bold' } + - docs_link_start = "".html_safe + - link_end = ''.html_safe + %p.gl-pl-6 + = s_("GitLabPages|When enabled, all attempts to visit your website through HTTP are automatically redirected to HTTPS using a response with status code 301. Requires a valid certificate for all domains. %{docs_link_start}Learn more.%{link_end}").html_safe % { docs_link_start: docs_link_start, link_end: link_end } .gl-mt-3 = f.submit s_('GitLabPages|Save changes'), class: 'btn btn-confirm gl-button' diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 37d31515307..b99294f504c 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -97,7 +97,7 @@ - if issuable_sidebar.dig(:current_user, :can_move) .block.js-sidebar-move-issue-block .sidebar-collapsed-icon{ data: { toggle: 'tooltip', placement: 'left', container: 'body', boundary: 'viewport' }, title: _('Move issue') } - = custom_icon('icon_arrow_right') + = sprite_icon('long-arrow') .dropdown.sidebar-move-issue-dropdown.hide-collapsed %button.gl-button.btn.btn-default.btn-block.js-sidebar-dropdown-toggle.js-move-issue{ type: 'button', data: { toggle: 'dropdown', display: 'static', track_label: "right_sidebar", track_property: "move_issue", track_action: "click_button", track_value: "" } } diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 92319a6cfab..d110e1994aa 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -10425,7 +10425,6 @@ Snapshot. | `recordedAt` | [`Time!`](#time) | Time the snapshot was recorded. | | `runnerConfigured` | [`Boolean!`](#boolean) | At least one runner was used. | | `sastEnabledCount` | [`Int`](#int) | Total number of projects with enabled SAST. | -| `securityScanSucceeded` **{warning-solid}** | [`Boolean!`](#boolean) | **Deprecated** in 14.1. Substituted with specific security metrics. Always false. | | `startTime` | [`Time!`](#time) | Start time for the snapshot where the data points were collected. | | `totalProjectsCount` | [`Int`](#int) | Total number of projects. | | `vulnerabilityManagementUsedCount` | [`Int`](#int) | Total number of projects with vulnerability management used at least once. | @@ -16064,6 +16063,7 @@ Represents a URL related to a security training. | Name | Type | Description | | ---- | ---- | ----------- | +| `identifier` | [`String`](#string) | Name of the vulnerability identifier. | | `name` | [`String`](#string) | Name of the training provider. | | `status` | [`TrainingUrlRequestStatus`](#trainingurlrequeststatus) | Status of the request to training provider. | | `url` | [`String`](#string) | URL of the link for security training content. | diff --git a/doc/development/database/keyset_pagination.md b/doc/development/database/keyset_pagination.md index 4f0b353a37f..88928feb927 100644 --- a/doc/development/database/keyset_pagination.md +++ b/doc/development/database/keyset_pagination.md @@ -166,7 +166,7 @@ These order objects can be defined in the model classes as normal ActiveRecord s Consider the following scope: ```ruby -scope = Issue.where(project_id: 10).order(Gitlab::Database.nulls_last_order('relative_position', 'DESC')) +scope = Issue.where(project_id: 10).order(Issue.arel_table[:relative_position].desc.nulls_last) # SELECT "issues".* FROM "issues" WHERE "issues"."project_id" = 10 ORDER BY relative_position DESC NULLS LAST scope.keyset_paginate # raises: Gitlab::Pagination::Keyset::UnsupportedScopeOrder: The order on the scope does not support keyset pagination @@ -190,8 +190,8 @@ order = Gitlab::Pagination::Keyset::Order.build([ Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'relative_position', column_expression: Issue.arel_table[:relative_position], - order_expression: Gitlab::Database.nulls_last_order('relative_position', 'DESC'), - reversed_order_expression: Gitlab::Database.nulls_first_order('relative_position', 'ASC'), + order_expression: Issue.arel_table[:relative_position].desc.nulls_last, + reversed_order_expression: Issue.arel_table[:relative_position].asc.nulls_first, nullable: :nulls_last, order_direction: :desc, distinct: false diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 3397fffeeea..2282a7d876e 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -169,6 +169,7 @@ The following table lists project permissions available for each role: | [Projects](project/index.md):
Delete project | | | | | ✓ | | [Projects](project/index.md):
Disable notification emails | | | | | ✓ | | [Projects](project/index.md):
Transfer project to another namespace | | | | | ✓ | +| [Projects](project/index.md): View [Usage Quotas](usage_quotas.md) page | | | | ✓ | ✓ | | [Repository](project/repository/index.md):
Pull project code | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | [Repository](project/repository/index.md):
View project code | ✓ (*1*) | ✓ | ✓ | ✓ | ✓ | | [Repository](project/repository/index.md):
View a commit status | | ✓ | ✓ | ✓ | ✓ | @@ -395,7 +396,6 @@ The following table lists group permissions available for each role: | View [Group DevOps Adoption](group/devops_adoption/index.md) | | ✓ | ✓ | ✓ | ✓ | | View metrics dashboard annotations | | ✓ | ✓ | ✓ | ✓ | | View [Productivity analytics](analytics/productivity_analytics.md) | | ✓ | ✓ | ✓ | ✓ | -| View Usage quota data | | ✓ | ✓ | ✓ | ✓ | | Create and edit [group wiki](project/wiki/group.md) pages | | | ✓ | ✓ | ✓ | | Create project in group | | | ✓ (3)(5) | ✓ (3) | ✓ (3) | | Create/edit/delete group milestones | | | ✓ | ✓ | ✓ | @@ -425,7 +425,7 @@ The following table lists group permissions available for each role: | Share (invite) groups with groups | | | | | ✓ | | View 2FA status of members | | | | | ✓ | | View [Billing](../subscriptions/gitlab_com/index.md#view-your-gitlab-saas-subscription) | | | | | ✓ (4) | -| View [Usage Quotas](usage_quotas.md) Page | | | | ✓ | ✓ (4) | +| View group [Usage Quotas](usage_quotas.md) page | | | | | ✓ (4) | | Manage group runners | | | | | ✓ | diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index b4edc363a59..77d7d7e19c1 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -161,24 +161,6 @@ module Gitlab end end - def self.nulls_order(field, direction = :asc, nulls_order = :nulls_last) - raise ArgumentError unless [:nulls_last, :nulls_first].include?(nulls_order) - raise ArgumentError unless [:asc, :desc].include?(direction) - - case nulls_order - when :nulls_last then nulls_last_order(field, direction) - when :nulls_first then nulls_first_order(field, direction) - end - end - - def self.nulls_last_order(field, direction = 'ASC') - Arel.sql("#{field} #{direction} NULLS LAST") - end - - def self.nulls_first_order(field, direction = 'ASC') - Arel.sql("#{field} #{direction} NULLS FIRST") - end - def self.random "RANDOM()" end diff --git a/lib/gitlab/graphql/pagination/keyset/connection.rb b/lib/gitlab/graphql/pagination/keyset/connection.rb index 61903c566f0..c284160e539 100644 --- a/lib/gitlab/graphql/pagination/keyset/connection.rb +++ b/lib/gitlab/graphql/pagination/keyset/connection.rb @@ -14,10 +14,6 @@ # Issue.order(created_at: :asc).order(:id) # Issue.order(due_date: :asc) # -# You can also use `Gitlab::Database.nulls_last_order`: -# -# Issue.reorder(::Gitlab::Database.nulls_last_order('due_date', 'DESC')) -# # It will tolerate non-attribute ordering, but only attributes determine the cursor. # For example, this is legitimate: # diff --git a/lib/gitlab/import_export/json/streaming_serializer.rb b/lib/gitlab/import_export/json/streaming_serializer.rb index d646a1a8207..ebabf537ce5 100644 --- a/lib/gitlab/import_export/json/streaming_serializer.rb +++ b/lib/gitlab/import_export/json/streaming_serializer.rb @@ -145,8 +145,8 @@ module Gitlab arel_order_classes = ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::AREL_ORDER_CLASSES.invert reverse_direction = ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::REVERSED_ORDER_DIRECTIONS[direction] reverse_nulls_position = ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::REVERSED_NULL_POSITIONS[nulls_position] - order_expression = ::Gitlab::Database.nulls_order(column, direction, nulls_position) - reverse_order_expression = ::Gitlab::Database.nulls_order(column, reverse_direction, reverse_nulls_position) + order_expression = arel_table[column].public_send(direction).public_send(nulls_position) # rubocop:disable GitlabSecurity/PublicSend + reverse_order_expression = arel_table[column].public_send(reverse_direction).public_send(reverse_nulls_position) # rubocop:disable GitlabSecurity/PublicSend ::Gitlab::Pagination::Keyset::Order.build([ ::Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( diff --git a/lib/gitlab/relative_positioning/item_context.rb b/lib/gitlab/relative_positioning/item_context.rb index 98e52e8e767..ac0598d8d34 100644 --- a/lib/gitlab/relative_positioning/item_context.rb +++ b/lib/gitlab/relative_positioning/item_context.rb @@ -84,7 +84,7 @@ module Gitlab # MAX(relative_position) without the GROUP BY, due to index usage: # https://gitlab.com/gitlab-org/gitlab-foss/issues/54276#note_119340977 relation = scoped_items - .order(Gitlab::Database.nulls_last_order('position', 'DESC')) + .order(Arel.sql('position').desc.nulls_last) .group(grouping_column) .limit(1) @@ -101,7 +101,7 @@ module Gitlab def max_sibling sib = relative_siblings - .order(Gitlab::Database.nulls_last_order('relative_position', 'DESC')) + .order(model_class.arel_table[:relative_position].desc.nulls_last) .first neighbour(sib) @@ -109,7 +109,7 @@ module Gitlab def min_sibling sib = relative_siblings - .order(Gitlab::Database.nulls_last_order('relative_position', 'ASC')) + .order(model_class.arel_table[:relative_position].asc.nulls_last) .first neighbour(sib) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 74d7d284a28..9cd750bb0bf 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -13288,7 +13288,7 @@ msgstr "" msgid "Display source" msgstr "" -msgid "Display time tracking in issues in total hours only." +msgid "Display time tracking in issues in total hours only. %{link_start}What is time tracking?%{link_end}" msgstr "" msgid "Do not display content for customer experience improvement and offers from third parties" @@ -42466,9 +42466,6 @@ msgstr "" msgid "What is squashing?" msgstr "" -msgid "What is time tracking?" -msgstr "" - msgid "What is your job title? (optional)" msgstr "" diff --git a/spec/frontend/badges/components/badge_spec.js b/spec/frontend/badges/components/badge_spec.js index 2310fb8bd8e..fe4cf8ce8eb 100644 --- a/spec/frontend/badges/components/badge_spec.js +++ b/spec/frontend/badges/components/badge_spec.js @@ -89,11 +89,9 @@ describe('Badge component', () => { }); describe('behavior', () => { - beforeEach((done) => { + beforeEach(() => { setFixtures('
'); - createComponent({ ...dummyProps }, '#dummy-element') - .then(done) - .catch(done.fail); + return createComponent({ ...dummyProps }, '#dummy-element'); }); it('shows a badge image after loading', () => { diff --git a/spec/frontend/badges/store/actions_spec.js b/spec/frontend/badges/store/actions_spec.js index 75699f24463..02e1b8e65e4 100644 --- a/spec/frontend/badges/store/actions_spec.js +++ b/spec/frontend/badges/store/actions_spec.js @@ -33,41 +33,38 @@ describe('Badges store actions', () => { }); describe('requestNewBadge', () => { - it('commits REQUEST_NEW_BADGE', (done) => { - testAction( + it('commits REQUEST_NEW_BADGE', () => { + return testAction( actions.requestNewBadge, null, state, [{ type: mutationTypes.REQUEST_NEW_BADGE }], [], - done, ); }); }); describe('receiveNewBadge', () => { - it('commits RECEIVE_NEW_BADGE', (done) => { + it('commits RECEIVE_NEW_BADGE', () => { const newBadge = createDummyBadge(); - testAction( + return testAction( actions.receiveNewBadge, newBadge, state, [{ type: mutationTypes.RECEIVE_NEW_BADGE, payload: newBadge }], [], - done, ); }); }); describe('receiveNewBadgeError', () => { - it('commits RECEIVE_NEW_BADGE_ERROR', (done) => { - testAction( + it('commits RECEIVE_NEW_BADGE_ERROR', () => { + return testAction( actions.receiveNewBadgeError, null, state, [{ type: mutationTypes.RECEIVE_NEW_BADGE_ERROR }], [], - done, ); }); }); @@ -87,7 +84,7 @@ describe('Badges store actions', () => { }; }); - it('dispatches requestNewBadge and receiveNewBadge for successful response', (done) => { + it('dispatches requestNewBadge and receiveNewBadge for successful response', async () => { const dummyResponse = createDummyBadgeResponse(); endpointMock.replyOnce((req) => { @@ -105,16 +102,12 @@ describe('Badges store actions', () => { }); const dummyBadge = transformBackendBadge(dummyResponse); - actions - .addBadge({ state, dispatch }) - .then(() => { - expect(dispatch.mock.calls).toEqual([['receiveNewBadge', dummyBadge]]); - }) - .then(done) - .catch(done.fail); + + await actions.addBadge({ state, dispatch }); + expect(dispatch.mock.calls).toEqual([['receiveNewBadge', dummyBadge]]); }); - it('dispatches requestNewBadge and receiveNewBadgeError for error response', (done) => { + it('dispatches requestNewBadge and receiveNewBadgeError for error response', async () => { endpointMock.replyOnce((req) => { expect(req.data).toBe( JSON.stringify({ @@ -129,52 +122,43 @@ describe('Badges store actions', () => { return [500, '']; }); - actions - .addBadge({ state, dispatch }) - .then(() => done.fail('Expected Ajax call to fail!')) - .catch(() => { - expect(dispatch.mock.calls).toEqual([['receiveNewBadgeError']]); - }) - .then(done) - .catch(done.fail); + await expect(actions.addBadge({ state, dispatch })).rejects.toThrow(); + expect(dispatch.mock.calls).toEqual([['receiveNewBadgeError']]); }); }); describe('requestDeleteBadge', () => { - it('commits REQUEST_DELETE_BADGE', (done) => { - testAction( + it('commits REQUEST_DELETE_BADGE', () => { + return testAction( actions.requestDeleteBadge, badgeId, state, [{ type: mutationTypes.REQUEST_DELETE_BADGE, payload: badgeId }], [], - done, ); }); }); describe('receiveDeleteBadge', () => { - it('commits RECEIVE_DELETE_BADGE', (done) => { - testAction( + it('commits RECEIVE_DELETE_BADGE', () => { + return testAction( actions.receiveDeleteBadge, badgeId, state, [{ type: mutationTypes.RECEIVE_DELETE_BADGE, payload: badgeId }], [], - done, ); }); }); describe('receiveDeleteBadgeError', () => { - it('commits RECEIVE_DELETE_BADGE_ERROR', (done) => { - testAction( + it('commits RECEIVE_DELETE_BADGE_ERROR', () => { + return testAction( actions.receiveDeleteBadgeError, badgeId, state, [{ type: mutationTypes.RECEIVE_DELETE_BADGE_ERROR, payload: badgeId }], [], - done, ); }); }); @@ -188,91 +172,76 @@ describe('Badges store actions', () => { dispatch = jest.fn(); }); - it('dispatches requestDeleteBadge and receiveDeleteBadge for successful response', (done) => { + it('dispatches requestDeleteBadge and receiveDeleteBadge for successful response', async () => { endpointMock.replyOnce(() => { expect(dispatch.mock.calls).toEqual([['requestDeleteBadge', badgeId]]); dispatch.mockClear(); return [200, '']; }); - actions - .deleteBadge({ state, dispatch }, { id: badgeId }) - .then(() => { - expect(dispatch.mock.calls).toEqual([['receiveDeleteBadge', badgeId]]); - }) - .then(done) - .catch(done.fail); + await actions.deleteBadge({ state, dispatch }, { id: badgeId }); + expect(dispatch.mock.calls).toEqual([['receiveDeleteBadge', badgeId]]); }); - it('dispatches requestDeleteBadge and receiveDeleteBadgeError for error response', (done) => { + it('dispatches requestDeleteBadge and receiveDeleteBadgeError for error response', async () => { endpointMock.replyOnce(() => { expect(dispatch.mock.calls).toEqual([['requestDeleteBadge', badgeId]]); dispatch.mockClear(); return [500, '']; }); - actions - .deleteBadge({ state, dispatch }, { id: badgeId }) - .then(() => done.fail('Expected Ajax call to fail!')) - .catch(() => { - expect(dispatch.mock.calls).toEqual([['receiveDeleteBadgeError', badgeId]]); - }) - .then(done) - .catch(done.fail); + await expect(actions.deleteBadge({ state, dispatch }, { id: badgeId })).rejects.toThrow(); + expect(dispatch.mock.calls).toEqual([['receiveDeleteBadgeError', badgeId]]); }); }); describe('editBadge', () => { - it('commits START_EDITING', (done) => { + it('commits START_EDITING', () => { const dummyBadge = createDummyBadge(); - testAction( + return testAction( actions.editBadge, dummyBadge, state, [{ type: mutationTypes.START_EDITING, payload: dummyBadge }], [], - done, ); }); }); describe('requestLoadBadges', () => { - it('commits REQUEST_LOAD_BADGES', (done) => { + it('commits REQUEST_LOAD_BADGES', () => { const dummyData = 'this is not real data'; - testAction( + return testAction( actions.requestLoadBadges, dummyData, state, [{ type: mutationTypes.REQUEST_LOAD_BADGES, payload: dummyData }], [], - done, ); }); }); describe('receiveLoadBadges', () => { - it('commits RECEIVE_LOAD_BADGES', (done) => { + it('commits RECEIVE_LOAD_BADGES', () => { const badges = dummyBadges; - testAction( + return testAction( actions.receiveLoadBadges, badges, state, [{ type: mutationTypes.RECEIVE_LOAD_BADGES, payload: badges }], [], - done, ); }); }); describe('receiveLoadBadgesError', () => { - it('commits RECEIVE_LOAD_BADGES_ERROR', (done) => { - testAction( + it('commits RECEIVE_LOAD_BADGES_ERROR', () => { + return testAction( actions.receiveLoadBadgesError, null, state, [{ type: mutationTypes.RECEIVE_LOAD_BADGES_ERROR }], [], - done, ); }); }); @@ -286,7 +255,7 @@ describe('Badges store actions', () => { dispatch = jest.fn(); }); - it('dispatches requestLoadBadges and receiveLoadBadges for successful response', (done) => { + it('dispatches requestLoadBadges and receiveLoadBadges for successful response', async () => { const dummyData = 'this is just some data'; const dummyReponse = [ createDummyBadgeResponse(), @@ -299,18 +268,13 @@ describe('Badges store actions', () => { return [200, dummyReponse]; }); - actions - .loadBadges({ state, dispatch }, dummyData) - .then(() => { - const badges = dummyReponse.map(transformBackendBadge); + await actions.loadBadges({ state, dispatch }, dummyData); + const badges = dummyReponse.map(transformBackendBadge); - expect(dispatch.mock.calls).toEqual([['receiveLoadBadges', badges]]); - }) - .then(done) - .catch(done.fail); + expect(dispatch.mock.calls).toEqual([['receiveLoadBadges', badges]]); }); - it('dispatches requestLoadBadges and receiveLoadBadgesError for error response', (done) => { + it('dispatches requestLoadBadges and receiveLoadBadgesError for error response', async () => { const dummyData = 'this is just some data'; endpointMock.replyOnce(() => { expect(dispatch.mock.calls).toEqual([['requestLoadBadges', dummyData]]); @@ -318,53 +282,44 @@ describe('Badges store actions', () => { return [500, '']; }); - actions - .loadBadges({ state, dispatch }, dummyData) - .then(() => done.fail('Expected Ajax call to fail!')) - .catch(() => { - expect(dispatch.mock.calls).toEqual([['receiveLoadBadgesError']]); - }) - .then(done) - .catch(done.fail); + await expect(actions.loadBadges({ state, dispatch }, dummyData)).rejects.toThrow(); + expect(dispatch.mock.calls).toEqual([['receiveLoadBadgesError']]); }); }); describe('requestRenderedBadge', () => { - it('commits REQUEST_RENDERED_BADGE', (done) => { - testAction( + it('commits REQUEST_RENDERED_BADGE', () => { + return testAction( actions.requestRenderedBadge, null, state, [{ type: mutationTypes.REQUEST_RENDERED_BADGE }], [], - done, ); }); }); describe('receiveRenderedBadge', () => { - it('commits RECEIVE_RENDERED_BADGE', (done) => { + it('commits RECEIVE_RENDERED_BADGE', () => { const dummyBadge = createDummyBadge(); - testAction( + return testAction( actions.receiveRenderedBadge, dummyBadge, state, [{ type: mutationTypes.RECEIVE_RENDERED_BADGE, payload: dummyBadge }], [], - done, ); }); }); describe('receiveRenderedBadgeError', () => { - it('commits RECEIVE_RENDERED_BADGE_ERROR', (done) => { - testAction( + it('commits RECEIVE_RENDERED_BADGE_ERROR', () => { + return testAction( actions.receiveRenderedBadgeError, null, state, [{ type: mutationTypes.RECEIVE_RENDERED_BADGE_ERROR }], [], - done, ); }); }); @@ -388,56 +343,41 @@ describe('Badges store actions', () => { dispatch = jest.fn(); }); - it('returns immediately if imageUrl is empty', (done) => { + it('returns immediately if imageUrl is empty', async () => { jest.spyOn(axios, 'get').mockImplementation(() => {}); badgeInForm.imageUrl = ''; - actions - .renderBadge({ state, dispatch }) - .then(() => { - expect(axios.get).not.toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); + await actions.renderBadge({ state, dispatch }); + expect(axios.get).not.toHaveBeenCalled(); }); - it('returns immediately if linkUrl is empty', (done) => { + it('returns immediately if linkUrl is empty', async () => { jest.spyOn(axios, 'get').mockImplementation(() => {}); badgeInForm.linkUrl = ''; - actions - .renderBadge({ state, dispatch }) - .then(() => { - expect(axios.get).not.toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); + await actions.renderBadge({ state, dispatch }); + expect(axios.get).not.toHaveBeenCalled(); }); - it('escapes user input', (done) => { + it('escapes user input', async () => { jest .spyOn(axios, 'get') .mockImplementation(() => Promise.resolve({ data: createDummyBadgeResponse() })); badgeInForm.imageUrl = '&make-sandwich=true'; badgeInForm.linkUrl = ''; - actions - .renderBadge({ state, dispatch }) - .then(() => { - expect(axios.get.mock.calls.length).toBe(1); - const url = axios.get.mock.calls[0][0]; + await actions.renderBadge({ state, dispatch }); + expect(axios.get.mock.calls.length).toBe(1); + const url = axios.get.mock.calls[0][0]; - expect(url).toMatch(new RegExp(`^${dummyEndpointUrl}/render?`)); - expect(url).toMatch( - new RegExp('\\?link_url=%3Cscript%3EI%20am%20dangerous!%3C%2Fscript%3E&'), - ); - expect(url).toMatch(new RegExp('&image_url=%26make-sandwich%3Dtrue$')); - }) - .then(done) - .catch(done.fail); + expect(url).toMatch(new RegExp(`^${dummyEndpointUrl}/render?`)); + expect(url).toMatch( + new RegExp('\\?link_url=%3Cscript%3EI%20am%20dangerous!%3C%2Fscript%3E&'), + ); + expect(url).toMatch(new RegExp('&image_url=%26make-sandwich%3Dtrue$')); }); - it('dispatches requestRenderedBadge and receiveRenderedBadge for successful response', (done) => { + it('dispatches requestRenderedBadge and receiveRenderedBadge for successful response', async () => { const dummyReponse = createDummyBadgeResponse(); endpointMock.replyOnce(() => { expect(dispatch.mock.calls).toEqual([['requestRenderedBadge']]); @@ -445,71 +385,57 @@ describe('Badges store actions', () => { return [200, dummyReponse]; }); - actions - .renderBadge({ state, dispatch }) - .then(() => { - const renderedBadge = transformBackendBadge(dummyReponse); + await actions.renderBadge({ state, dispatch }); + const renderedBadge = transformBackendBadge(dummyReponse); - expect(dispatch.mock.calls).toEqual([['receiveRenderedBadge', renderedBadge]]); - }) - .then(done) - .catch(done.fail); + expect(dispatch.mock.calls).toEqual([['receiveRenderedBadge', renderedBadge]]); }); - it('dispatches requestRenderedBadge and receiveRenderedBadgeError for error response', (done) => { + it('dispatches requestRenderedBadge and receiveRenderedBadgeError for error response', async () => { endpointMock.replyOnce(() => { expect(dispatch.mock.calls).toEqual([['requestRenderedBadge']]); dispatch.mockClear(); return [500, '']; }); - actions - .renderBadge({ state, dispatch }) - .then(() => done.fail('Expected Ajax call to fail!')) - .catch(() => { - expect(dispatch.mock.calls).toEqual([['receiveRenderedBadgeError']]); - }) - .then(done) - .catch(done.fail); + await expect(actions.renderBadge({ state, dispatch })).rejects.toThrow(); + expect(dispatch.mock.calls).toEqual([['receiveRenderedBadgeError']]); }); }); describe('requestUpdatedBadge', () => { - it('commits REQUEST_UPDATED_BADGE', (done) => { - testAction( + it('commits REQUEST_UPDATED_BADGE', () => { + return testAction( actions.requestUpdatedBadge, null, state, [{ type: mutationTypes.REQUEST_UPDATED_BADGE }], [], - done, ); }); }); describe('receiveUpdatedBadge', () => { - it('commits RECEIVE_UPDATED_BADGE', (done) => { + it('commits RECEIVE_UPDATED_BADGE', () => { const updatedBadge = createDummyBadge(); - testAction( + return testAction( actions.receiveUpdatedBadge, updatedBadge, state, [{ type: mutationTypes.RECEIVE_UPDATED_BADGE, payload: updatedBadge }], [], - done, ); }); }); describe('receiveUpdatedBadgeError', () => { - it('commits RECEIVE_UPDATED_BADGE_ERROR', (done) => { - testAction( + it('commits RECEIVE_UPDATED_BADGE_ERROR', () => { + return testAction( actions.receiveUpdatedBadgeError, null, state, [{ type: mutationTypes.RECEIVE_UPDATED_BADGE_ERROR }], [], - done, ); }); }); @@ -529,7 +455,7 @@ describe('Badges store actions', () => { dispatch = jest.fn(); }); - it('dispatches requestUpdatedBadge and receiveUpdatedBadge for successful response', (done) => { + it('dispatches requestUpdatedBadge and receiveUpdatedBadge for successful response', async () => { const dummyResponse = createDummyBadgeResponse(); endpointMock.replyOnce((req) => { @@ -547,16 +473,11 @@ describe('Badges store actions', () => { }); const updatedBadge = transformBackendBadge(dummyResponse); - actions - .saveBadge({ state, dispatch }) - .then(() => { - expect(dispatch.mock.calls).toEqual([['receiveUpdatedBadge', updatedBadge]]); - }) - .then(done) - .catch(done.fail); + await actions.saveBadge({ state, dispatch }); + expect(dispatch.mock.calls).toEqual([['receiveUpdatedBadge', updatedBadge]]); }); - it('dispatches requestUpdatedBadge and receiveUpdatedBadgeError for error response', (done) => { + it('dispatches requestUpdatedBadge and receiveUpdatedBadgeError for error response', async () => { endpointMock.replyOnce((req) => { expect(req.data).toBe( JSON.stringify({ @@ -571,53 +492,44 @@ describe('Badges store actions', () => { return [500, '']; }); - actions - .saveBadge({ state, dispatch }) - .then(() => done.fail('Expected Ajax call to fail!')) - .catch(() => { - expect(dispatch.mock.calls).toEqual([['receiveUpdatedBadgeError']]); - }) - .then(done) - .catch(done.fail); + await expect(actions.saveBadge({ state, dispatch })).rejects.toThrow(); + expect(dispatch.mock.calls).toEqual([['receiveUpdatedBadgeError']]); }); }); describe('stopEditing', () => { - it('commits STOP_EDITING', (done) => { - testAction( + it('commits STOP_EDITING', () => { + return testAction( actions.stopEditing, null, state, [{ type: mutationTypes.STOP_EDITING }], [], - done, ); }); }); describe('updateBadgeInForm', () => { - it('commits UPDATE_BADGE_IN_FORM', (done) => { + it('commits UPDATE_BADGE_IN_FORM', () => { const dummyBadge = createDummyBadge(); - testAction( + return testAction( actions.updateBadgeInForm, dummyBadge, state, [{ type: mutationTypes.UPDATE_BADGE_IN_FORM, payload: dummyBadge }], [], - done, ); }); describe('updateBadgeInModal', () => { - it('commits UPDATE_BADGE_IN_MODAL', (done) => { + it('commits UPDATE_BADGE_IN_MODAL', () => { const dummyBadge = createDummyBadge(); - testAction( + return testAction( actions.updateBadgeInModal, dummyBadge, state, [{ type: mutationTypes.UPDATE_BADGE_IN_MODAL, payload: dummyBadge }], [], - done, ); }); }); diff --git a/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js b/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js index b0e9e5dd00b..e9535d8cc12 100644 --- a/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js +++ b/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js @@ -29,53 +29,56 @@ describe('Batch comments store actions', () => { }); describe('addDraftToDiscussion', () => { - it('commits ADD_NEW_DRAFT if no errors returned', (done) => { + it('commits ADD_NEW_DRAFT if no errors returned', () => { res = { id: 1 }; mock.onAny().reply(200, res); - testAction( + return testAction( actions.addDraftToDiscussion, { endpoint: TEST_HOST, data: 'test' }, null, [{ type: 'ADD_NEW_DRAFT', payload: res }], [], - done, ); }); - it('does not commit ADD_NEW_DRAFT if errors returned', (done) => { + it('does not commit ADD_NEW_DRAFT if errors returned', () => { mock.onAny().reply(500); - testAction( + return testAction( actions.addDraftToDiscussion, { endpoint: TEST_HOST, data: 'test' }, null, [], [], - done, ); }); }); describe('createNewDraft', () => { - it('commits ADD_NEW_DRAFT if no errors returned', (done) => { + it('commits ADD_NEW_DRAFT if no errors returned', () => { res = { id: 1 }; mock.onAny().reply(200, res); - testAction( + return testAction( actions.createNewDraft, { endpoint: TEST_HOST, data: 'test' }, null, [{ type: 'ADD_NEW_DRAFT', payload: res }], [], - done, ); }); - it('does not commit ADD_NEW_DRAFT if errors returned', (done) => { + it('does not commit ADD_NEW_DRAFT if errors returned', () => { mock.onAny().reply(500); - testAction(actions.createNewDraft, { endpoint: TEST_HOST, data: 'test' }, null, [], [], done); + return testAction( + actions.createNewDraft, + { endpoint: TEST_HOST, data: 'test' }, + null, + [], + [], + ); }); }); @@ -90,7 +93,7 @@ describe('Batch comments store actions', () => { }; }); - it('commits DELETE_DRAFT if no errors returned', (done) => { + it('commits DELETE_DRAFT if no errors returned', () => { const commit = jest.fn(); const context = { getters, @@ -99,16 +102,12 @@ describe('Batch comments store actions', () => { res = { id: 1 }; mock.onAny().reply(200); - actions - .deleteDraft(context, { id: 1 }) - .then(() => { - expect(commit).toHaveBeenCalledWith('DELETE_DRAFT', 1); - }) - .then(done) - .catch(done.fail); + return actions.deleteDraft(context, { id: 1 }).then(() => { + expect(commit).toHaveBeenCalledWith('DELETE_DRAFT', 1); + }); }); - it('does not commit DELETE_DRAFT if errors returned', (done) => { + it('does not commit DELETE_DRAFT if errors returned', () => { const commit = jest.fn(); const context = { getters, @@ -116,13 +115,9 @@ describe('Batch comments store actions', () => { }; mock.onAny().reply(500); - actions - .deleteDraft(context, { id: 1 }) - .then(() => { - expect(commit).not.toHaveBeenCalledWith('DELETE_DRAFT', 1); - }) - .then(done) - .catch(done.fail); + return actions.deleteDraft(context, { id: 1 }).then(() => { + expect(commit).not.toHaveBeenCalledWith('DELETE_DRAFT', 1); + }); }); }); @@ -137,7 +132,7 @@ describe('Batch comments store actions', () => { }; }); - it('commits SET_BATCH_COMMENTS_DRAFTS with returned data', (done) => { + it('commits SET_BATCH_COMMENTS_DRAFTS with returned data', () => { const commit = jest.fn(); const dispatch = jest.fn(); const context = { @@ -151,14 +146,10 @@ describe('Batch comments store actions', () => { res = { id: 1 }; mock.onAny().reply(200, res); - actions - .fetchDrafts(context) - .then(() => { - expect(commit).toHaveBeenCalledWith('SET_BATCH_COMMENTS_DRAFTS', { id: 1 }); - expect(dispatch).toHaveBeenCalledWith('convertToDiscussion', '1', { root: true }); - }) - .then(done) - .catch(done.fail); + return actions.fetchDrafts(context).then(() => { + expect(commit).toHaveBeenCalledWith('SET_BATCH_COMMENTS_DRAFTS', { id: 1 }); + expect(dispatch).toHaveBeenCalledWith('convertToDiscussion', '1', { root: true }); + }); }); }); @@ -177,32 +168,24 @@ describe('Batch comments store actions', () => { rootGetters = { discussionsStructuredByLineCode: 'discussions' }; }); - it('dispatches actions & commits', (done) => { + it('dispatches actions & commits', () => { mock.onAny().reply(200); - actions - .publishReview({ dispatch, commit, getters, rootGetters }) - .then(() => { - expect(commit.mock.calls[0]).toEqual(['REQUEST_PUBLISH_REVIEW']); - expect(commit.mock.calls[1]).toEqual(['RECEIVE_PUBLISH_REVIEW_SUCCESS']); + return actions.publishReview({ dispatch, commit, getters, rootGetters }).then(() => { + expect(commit.mock.calls[0]).toEqual(['REQUEST_PUBLISH_REVIEW']); + expect(commit.mock.calls[1]).toEqual(['RECEIVE_PUBLISH_REVIEW_SUCCESS']); - expect(dispatch.mock.calls[0]).toEqual(['updateDiscussionsAfterPublish']); - }) - .then(done) - .catch(done.fail); + expect(dispatch.mock.calls[0]).toEqual(['updateDiscussionsAfterPublish']); + }); }); - it('dispatches error commits', (done) => { + it('dispatches error commits', () => { mock.onAny().reply(500); - actions - .publishReview({ dispatch, commit, getters, rootGetters }) - .then(() => { - expect(commit.mock.calls[0]).toEqual(['REQUEST_PUBLISH_REVIEW']); - expect(commit.mock.calls[1]).toEqual(['RECEIVE_PUBLISH_REVIEW_ERROR']); - }) - .then(done) - .catch(done.fail); + return actions.publishReview({ dispatch, commit, getters, rootGetters }).then(() => { + expect(commit.mock.calls[0]).toEqual(['REQUEST_PUBLISH_REVIEW']); + expect(commit.mock.calls[1]).toEqual(['RECEIVE_PUBLISH_REVIEW_ERROR']); + }); }); }); @@ -262,7 +245,7 @@ describe('Batch comments store actions', () => { }); describe('expandAllDiscussions', () => { - it('dispatches expandDiscussion for all drafts', (done) => { + it('dispatches expandDiscussion for all drafts', () => { const state = { drafts: [ { @@ -271,7 +254,7 @@ describe('Batch comments store actions', () => { ], }; - testAction( + return testAction( actions.expandAllDiscussions, null, state, @@ -282,7 +265,6 @@ describe('Batch comments store actions', () => { payload: { discussionId: '1' }, }, ], - done, ); }); }); diff --git a/spec/frontend/boards/stores/actions_spec.js b/spec/frontend/boards/stores/actions_spec.js index ad661a31556..eacf9db191e 100644 --- a/spec/frontend/boards/stores/actions_spec.js +++ b/spec/frontend/boards/stores/actions_spec.js @@ -166,31 +166,29 @@ describe('setFilters', () => { }); describe('performSearch', () => { - it('should dispatch setFilters, fetchLists and resetIssues action', (done) => { - testAction( + it('should dispatch setFilters, fetchLists and resetIssues action', () => { + return testAction( actions.performSearch, {}, {}, [], [{ type: 'setFilters', payload: {} }, { type: 'fetchLists' }, { type: 'resetIssues' }], - done, ); }); }); describe('setActiveId', () => { - it('should commit mutation SET_ACTIVE_ID', (done) => { + it('should commit mutation SET_ACTIVE_ID', () => { const state = { activeId: inactiveId, }; - testAction( + return testAction( actions.setActiveId, { id: 1, sidebarType: 'something' }, state, [{ type: types.SET_ACTIVE_ID, payload: { id: 1, sidebarType: 'something' } }], [], - done, ); }); }); @@ -219,10 +217,10 @@ describe('fetchLists', () => { const formattedLists = formatBoardLists(queryResponse.data.group.board.lists); - it('should commit mutations RECEIVE_BOARD_LISTS_SUCCESS on success', (done) => { + it('should commit mutations RECEIVE_BOARD_LISTS_SUCCESS on success', () => { jest.spyOn(gqlClient, 'query').mockResolvedValue(queryResponse); - testAction( + return testAction( actions.fetchLists, {}, state, @@ -233,14 +231,13 @@ describe('fetchLists', () => { }, ], [], - done, ); }); - it('should commit mutations RECEIVE_BOARD_LISTS_FAILURE on failure', (done) => { + it('should commit mutations RECEIVE_BOARD_LISTS_FAILURE on failure', () => { jest.spyOn(gqlClient, 'query').mockResolvedValue(Promise.reject()); - testAction( + return testAction( actions.fetchLists, {}, state, @@ -250,11 +247,10 @@ describe('fetchLists', () => { }, ], [], - done, ); }); - it('dispatch createList action when backlog list does not exist and is not hidden', (done) => { + it('dispatch createList action when backlog list does not exist and is not hidden', () => { queryResponse = { data: { group: { @@ -269,7 +265,7 @@ describe('fetchLists', () => { }; jest.spyOn(gqlClient, 'query').mockResolvedValue(queryResponse); - testAction( + return testAction( actions.fetchLists, {}, state, @@ -280,7 +276,6 @@ describe('fetchLists', () => { }, ], [{ type: 'createList', payload: { backlog: true } }], - done, ); }); @@ -951,10 +946,10 @@ describe('fetchItemsForList', () => { }); }); - it('should commit mutations REQUEST_ITEMS_FOR_LIST and RECEIVE_ITEMS_FOR_LIST_SUCCESS on success', (done) => { + it('should commit mutations REQUEST_ITEMS_FOR_LIST and RECEIVE_ITEMS_FOR_LIST_SUCCESS on success', () => { jest.spyOn(gqlClient, 'query').mockResolvedValue(queryResponse); - testAction( + return testAction( actions.fetchItemsForList, { listId }, state, @@ -973,14 +968,13 @@ describe('fetchItemsForList', () => { }, ], [], - done, ); }); - it('should commit mutations REQUEST_ITEMS_FOR_LIST and RECEIVE_ITEMS_FOR_LIST_FAILURE on failure', (done) => { + it('should commit mutations REQUEST_ITEMS_FOR_LIST and RECEIVE_ITEMS_FOR_LIST_FAILURE on failure', () => { jest.spyOn(gqlClient, 'query').mockResolvedValue(Promise.reject()); - testAction( + return testAction( actions.fetchItemsForList, { listId }, state, @@ -996,7 +990,6 @@ describe('fetchItemsForList', () => { { type: types.RECEIVE_ITEMS_FOR_LIST_FAILURE, payload: listId }, ], [], - done, ); }); }); @@ -1398,8 +1391,8 @@ describe('setAssignees', () => { const node = { username: 'name' }; describe('when succeeds', () => { - it('calls the correct mutation with the correct values', (done) => { - testAction( + it('calls the correct mutation with the correct values', () => { + return testAction( actions.setAssignees, { assignees: [node], iid: '1' }, { commit: () => {} }, @@ -1410,7 +1403,6 @@ describe('setAssignees', () => { }, ], [], - done, ); }); }); @@ -1728,7 +1720,7 @@ describe('setActiveItemSubscribed', () => { projectPath: 'gitlab-org/gitlab-test', }; - it('should commit subscribed status', (done) => { + it('should commit subscribed status', () => { jest.spyOn(gqlClient, 'mutate').mockResolvedValue({ data: { updateIssuableSubscription: { @@ -1746,7 +1738,7 @@ describe('setActiveItemSubscribed', () => { value: subscribedState, }; - testAction( + return testAction( actions.setActiveItemSubscribed, input, { ...state, ...getters }, @@ -1757,7 +1749,6 @@ describe('setActiveItemSubscribed', () => { }, ], [], - done, ); }); @@ -1783,7 +1774,7 @@ describe('setActiveItemTitle', () => { projectPath: 'h/b', }; - it('should commit title after setting the issue', (done) => { + it('should commit title after setting the issue', () => { jest.spyOn(gqlClient, 'mutate').mockResolvedValue({ data: { updateIssuableTitle: { @@ -1801,7 +1792,7 @@ describe('setActiveItemTitle', () => { value: testTitle, }; - testAction( + return testAction( actions.setActiveItemTitle, input, { ...state, ...getters }, @@ -1812,7 +1803,6 @@ describe('setActiveItemTitle', () => { }, ], [], - done, ); }); @@ -1829,14 +1819,14 @@ describe('setActiveItemConfidential', () => { const state = { boardItems: { [mockIssue.id]: mockIssue } }; const getters = { activeBoardItem: mockIssue }; - it('set confidential value on board item', (done) => { + it('set confidential value on board item', () => { const payload = { itemId: getters.activeBoardItem.id, prop: 'confidential', value: true, }; - testAction( + return testAction( actions.setActiveItemConfidential, true, { ...state, ...getters }, @@ -1847,7 +1837,6 @@ describe('setActiveItemConfidential', () => { }, ], [], - done, ); }); }); @@ -1876,10 +1865,10 @@ describe('fetchGroupProjects', () => { }, }; - it('should commit mutations REQUEST_GROUP_PROJECTS and RECEIVE_GROUP_PROJECTS_SUCCESS on success', (done) => { + it('should commit mutations REQUEST_GROUP_PROJECTS and RECEIVE_GROUP_PROJECTS_SUCCESS on success', () => { jest.spyOn(gqlClient, 'query').mockResolvedValue(queryResponse); - testAction( + return testAction( actions.fetchGroupProjects, {}, state, @@ -1894,14 +1883,13 @@ describe('fetchGroupProjects', () => { }, ], [], - done, ); }); - it('should commit mutations REQUEST_GROUP_PROJECTS and RECEIVE_GROUP_PROJECTS_FAILURE on failure', (done) => { + it('should commit mutations REQUEST_GROUP_PROJECTS and RECEIVE_GROUP_PROJECTS_FAILURE on failure', () => { jest.spyOn(gqlClient, 'query').mockRejectedValue(); - testAction( + return testAction( actions.fetchGroupProjects, {}, state, @@ -1915,16 +1903,15 @@ describe('fetchGroupProjects', () => { }, ], [], - done, ); }); }); describe('setSelectedProject', () => { - it('should commit mutation SET_SELECTED_PROJECT', (done) => { + it('should commit mutation SET_SELECTED_PROJECT', () => { const project = mockGroupProjects[0]; - testAction( + return testAction( actions.setSelectedProject, project, {}, @@ -1935,7 +1922,6 @@ describe('setSelectedProject', () => { }, ], [], - done, ); }); }); diff --git a/spec/frontend/captcha/apollo_captcha_link_spec.js b/spec/frontend/captcha/apollo_captcha_link_spec.js index eab52344d1f..cd32e63d00c 100644 --- a/spec/frontend/captcha/apollo_captcha_link_spec.js +++ b/spec/frontend/captcha/apollo_captcha_link_spec.js @@ -95,70 +95,82 @@ describe('apolloCaptchaLink', () => { return { operationName: 'operation', variables: {}, setContext: mockContext }; } - it('successful responses are passed through', (done) => { + it('successful responses are passed through', () => { setupLink(SUCCESS_RESPONSE); - link.request(mockOperation()).subscribe((result) => { - expect(result).toEqual(SUCCESS_RESPONSE); - expect(mockLinkImplementation).toHaveBeenCalledTimes(1); - expect(waitForCaptchaToBeSolved).not.toHaveBeenCalled(); - done(); + + return new Promise((resolve) => { + link.request(mockOperation()).subscribe((result) => { + expect(result).toEqual(SUCCESS_RESPONSE); + expect(mockLinkImplementation).toHaveBeenCalledTimes(1); + expect(waitForCaptchaToBeSolved).not.toHaveBeenCalled(); + resolve(); + }); }); }); - it('non-spam related errors are passed through', (done) => { + it('non-spam related errors are passed through', () => { setupLink(NON_CAPTCHA_ERROR_RESPONSE); - link.request(mockOperation()).subscribe((result) => { - expect(result).toEqual(NON_CAPTCHA_ERROR_RESPONSE); - expect(mockLinkImplementation).toHaveBeenCalledTimes(1); - expect(mockContext).not.toHaveBeenCalled(); - expect(waitForCaptchaToBeSolved).not.toHaveBeenCalled(); - done(); + + return new Promise((resolve) => { + link.request(mockOperation()).subscribe((result) => { + expect(result).toEqual(NON_CAPTCHA_ERROR_RESPONSE); + expect(mockLinkImplementation).toHaveBeenCalledTimes(1); + expect(mockContext).not.toHaveBeenCalled(); + expect(waitForCaptchaToBeSolved).not.toHaveBeenCalled(); + resolve(); + }); }); }); - it('unresolvable spam errors are passed through', (done) => { + it('unresolvable spam errors are passed through', () => { setupLink(SPAM_ERROR_RESPONSE); - link.request(mockOperation()).subscribe((result) => { - expect(result).toEqual(SPAM_ERROR_RESPONSE); - expect(mockLinkImplementation).toHaveBeenCalledTimes(1); - expect(mockContext).not.toHaveBeenCalled(); - expect(waitForCaptchaToBeSolved).not.toHaveBeenCalled(); - done(); + return new Promise((resolve) => { + link.request(mockOperation()).subscribe((result) => { + expect(result).toEqual(SPAM_ERROR_RESPONSE); + expect(mockLinkImplementation).toHaveBeenCalledTimes(1); + expect(mockContext).not.toHaveBeenCalled(); + expect(waitForCaptchaToBeSolved).not.toHaveBeenCalled(); + resolve(); + }); }); }); describe('resolvable spam errors', () => { - it('re-submits request with spam headers if the captcha modal was solved correctly', (done) => { + it('re-submits request with spam headers if the captcha modal was solved correctly', () => { waitForCaptchaToBeSolved.mockResolvedValue(CAPTCHA_RESPONSE); setupLink(CAPTCHA_ERROR_RESPONSE, SUCCESS_RESPONSE); - link.request(mockOperation()).subscribe((result) => { - expect(result).toEqual(SUCCESS_RESPONSE); - expect(waitForCaptchaToBeSolved).toHaveBeenCalledWith(CAPTCHA_SITE_KEY); - expect(mockContext).toHaveBeenCalledWith({ - headers: { - 'X-GitLab-Captcha-Response': CAPTCHA_RESPONSE, - 'X-GitLab-Spam-Log-Id': SPAM_LOG_ID, - }, + return new Promise((resolve) => { + link.request(mockOperation()).subscribe((result) => { + expect(result).toEqual(SUCCESS_RESPONSE); + expect(waitForCaptchaToBeSolved).toHaveBeenCalledWith(CAPTCHA_SITE_KEY); + expect(mockContext).toHaveBeenCalledWith({ + headers: { + 'X-GitLab-Captcha-Response': CAPTCHA_RESPONSE, + 'X-GitLab-Spam-Log-Id': SPAM_LOG_ID, + }, + }); + expect(mockLinkImplementation).toHaveBeenCalledTimes(2); + resolve(); }); - expect(mockLinkImplementation).toHaveBeenCalledTimes(2); - done(); }); }); - it('throws error if the captcha modal was not solved correctly', (done) => { + it('throws error if the captcha modal was not solved correctly', () => { const error = new UnsolvedCaptchaError(); waitForCaptchaToBeSolved.mockRejectedValue(error); setupLink(CAPTCHA_ERROR_RESPONSE, SUCCESS_RESPONSE); - link.request(mockOperation()).subscribe({ - next: done.catch, - error: (result) => { - expect(result).toEqual(error); - expect(waitForCaptchaToBeSolved).toHaveBeenCalledWith(CAPTCHA_SITE_KEY); - expect(mockContext).not.toHaveBeenCalled(); - expect(mockLinkImplementation).toHaveBeenCalledTimes(1); - done(); - }, + return new Promise((resolve, reject) => { + link.request(mockOperation()).subscribe({ + next: reject, + error: (result) => { + expect(result).toEqual(error); + expect(waitForCaptchaToBeSolved).toHaveBeenCalledWith(CAPTCHA_SITE_KEY); + expect(mockContext).not.toHaveBeenCalled(); + expect(mockLinkImplementation).toHaveBeenCalledTimes(1); + resolve(); + }, + }); }); }); }); diff --git a/spec/frontend/ci_variable_list/store/actions_spec.js b/spec/frontend/ci_variable_list/store/actions_spec.js index 426e6cae8fb..eb31fcd3ef4 100644 --- a/spec/frontend/ci_variable_list/store/actions_spec.js +++ b/spec/frontend/ci_variable_list/store/actions_spec.js @@ -86,10 +86,10 @@ describe('CI variable list store actions', () => { }); describe('deleteVariable', () => { - it('dispatch correct actions on successful deleted variable', (done) => { + it('dispatch correct actions on successful deleted variable', () => { mock.onPatch(state.endpoint).reply(200); - testAction( + return testAction( actions.deleteVariable, {}, state, @@ -99,16 +99,13 @@ describe('CI variable list store actions', () => { { type: 'receiveDeleteVariableSuccess' }, { type: 'fetchVariables' }, ], - () => { - done(); - }, ); }); - it('should show flash error and set error in state on delete failure', (done) => { + it('should show flash error and set error in state on delete failure', async () => { mock.onPatch(state.endpoint).reply(500, ''); - testAction( + await testAction( actions.deleteVariable, {}, state, @@ -120,19 +117,16 @@ describe('CI variable list store actions', () => { payload: payloadError, }, ], - () => { - expect(createFlash).toHaveBeenCalled(); - done(); - }, ); + expect(createFlash).toHaveBeenCalled(); }); }); describe('updateVariable', () => { - it('dispatch correct actions on successful updated variable', (done) => { + it('dispatch correct actions on successful updated variable', () => { mock.onPatch(state.endpoint).reply(200); - testAction( + return testAction( actions.updateVariable, {}, state, @@ -142,16 +136,13 @@ describe('CI variable list store actions', () => { { type: 'receiveUpdateVariableSuccess' }, { type: 'fetchVariables' }, ], - () => { - done(); - }, ); }); - it('should show flash error and set error in state on update failure', (done) => { + it('should show flash error and set error in state on update failure', async () => { mock.onPatch(state.endpoint).reply(500, ''); - testAction( + await testAction( actions.updateVariable, mockVariable, state, @@ -163,19 +154,16 @@ describe('CI variable list store actions', () => { payload: payloadError, }, ], - () => { - expect(createFlash).toHaveBeenCalled(); - done(); - }, ); + expect(createFlash).toHaveBeenCalled(); }); }); describe('addVariable', () => { - it('dispatch correct actions on successful added variable', (done) => { + it('dispatch correct actions on successful added variable', () => { mock.onPatch(state.endpoint).reply(200); - testAction( + return testAction( actions.addVariable, {}, state, @@ -185,16 +173,13 @@ describe('CI variable list store actions', () => { { type: 'receiveAddVariableSuccess' }, { type: 'fetchVariables' }, ], - () => { - done(); - }, ); }); - it('should show flash error and set error in state on add failure', (done) => { + it('should show flash error and set error in state on add failure', async () => { mock.onPatch(state.endpoint).reply(500, ''); - testAction( + await testAction( actions.addVariable, {}, state, @@ -206,19 +191,16 @@ describe('CI variable list store actions', () => { payload: payloadError, }, ], - () => { - expect(createFlash).toHaveBeenCalled(); - done(); - }, ); + expect(createFlash).toHaveBeenCalled(); }); }); describe('fetchVariables', () => { - it('dispatch correct actions on fetchVariables', (done) => { + it('dispatch correct actions on fetchVariables', () => { mock.onGet(state.endpoint).reply(200, { variables: mockData.mockVariables }); - testAction( + return testAction( actions.fetchVariables, {}, state, @@ -230,29 +212,24 @@ describe('CI variable list store actions', () => { payload: prepareDataForDisplay(mockData.mockVariables), }, ], - () => { - done(); - }, ); }); - it('should show flash error and set error in state on fetch variables failure', (done) => { + it('should show flash error and set error in state on fetch variables failure', async () => { mock.onGet(state.endpoint).reply(500); - testAction(actions.fetchVariables, {}, state, [], [{ type: 'requestVariables' }], () => { - expect(createFlash).toHaveBeenCalledWith({ - message: 'There was an error fetching the variables.', - }); - done(); + await testAction(actions.fetchVariables, {}, state, [], [{ type: 'requestVariables' }]); + expect(createFlash).toHaveBeenCalledWith({ + message: 'There was an error fetching the variables.', }); }); }); describe('fetchEnvironments', () => { - it('dispatch correct actions on fetchEnvironments', (done) => { + it('dispatch correct actions on fetchEnvironments', () => { Api.environments = jest.fn().mockResolvedValue({ data: mockData.mockEnvironments }); - testAction( + return testAction( actions.fetchEnvironments, {}, state, @@ -264,28 +241,17 @@ describe('CI variable list store actions', () => { payload: prepareEnvironments(mockData.mockEnvironments), }, ], - () => { - done(); - }, ); }); - it('should show flash error and set error in state on fetch environments failure', (done) => { + it('should show flash error and set error in state on fetch environments failure', async () => { Api.environments = jest.fn().mockRejectedValue(); - testAction( - actions.fetchEnvironments, - {}, - state, - [], - [{ type: 'requestEnvironments' }], - () => { - expect(createFlash).toHaveBeenCalledWith({ - message: 'There was an error fetching the environments information.', - }); - done(); - }, - ); + await testAction(actions.fetchEnvironments, {}, state, [], [{ type: 'requestEnvironments' }]); + + expect(createFlash).toHaveBeenCalledWith({ + message: 'There was an error fetching the environments information.', + }); }); }); diff --git a/spec/frontend/clusters_list/store/actions_spec.js b/spec/frontend/clusters_list/store/actions_spec.js index f4b69053e14..7663f329b3f 100644 --- a/spec/frontend/clusters_list/store/actions_spec.js +++ b/spec/frontend/clusters_list/store/actions_spec.js @@ -24,14 +24,12 @@ describe('Clusters store actions', () => { captureException.mockRestore(); }); - it('should report sentry error', (done) => { + it('should report sentry error', async () => { const sentryError = new Error('New Sentry Error'); const tag = 'sentryErrorTag'; - testAction(actions.reportSentryError, { error: sentryError, tag }, {}, [], [], () => { - expect(captureException).toHaveBeenCalledWith(sentryError); - done(); - }); + await testAction(actions.reportSentryError, { error: sentryError, tag }, {}, [], []); + expect(captureException).toHaveBeenCalledWith(sentryError); }); }); @@ -62,10 +60,10 @@ describe('Clusters store actions', () => { afterEach(() => mock.restore()); - it('should commit SET_CLUSTERS_DATA with received response', (done) => { + it('should commit SET_CLUSTERS_DATA with received response', () => { mock.onGet().reply(200, apiData, headers); - testAction( + return testAction( actions.fetchClusters, { endpoint: apiData.endpoint }, {}, @@ -75,14 +73,13 @@ describe('Clusters store actions', () => { { type: types.SET_LOADING_CLUSTERS, payload: false }, ], [], - () => done(), ); }); - it('should show flash on API error', (done) => { + it('should show flash on API error', async () => { mock.onGet().reply(400, 'Not Found'); - testAction( + await testAction( actions.fetchClusters, { endpoint: apiData.endpoint }, {}, @@ -100,13 +97,10 @@ describe('Clusters store actions', () => { }, }, ], - () => { - expect(createFlash).toHaveBeenCalledWith({ - message: expect.stringMatching('error'), - }); - done(); - }, ); + expect(createFlash).toHaveBeenCalledWith({ + message: expect.stringMatching('error'), + }); }); describe('multiple api requests', () => { @@ -128,8 +122,8 @@ describe('Clusters store actions', () => { pollStop.mockRestore(); }); - it('should stop polling after MAX Requests', (done) => { - testAction( + it('should stop polling after MAX Requests', async () => { + await testAction( actions.fetchClusters, { endpoint: apiData.endpoint }, {}, @@ -139,47 +133,43 @@ describe('Clusters store actions', () => { { type: types.SET_LOADING_CLUSTERS, payload: false }, ], [], - () => { - expect(pollRequest).toHaveBeenCalledTimes(1); + ); + expect(pollRequest).toHaveBeenCalledTimes(1); + expect(pollStop).toHaveBeenCalledTimes(0); + jest.advanceTimersByTime(pollInterval); + + return waitForPromises() + .then(() => { + expect(pollRequest).toHaveBeenCalledTimes(2); expect(pollStop).toHaveBeenCalledTimes(0); jest.advanceTimersByTime(pollInterval); - - waitForPromises() - .then(() => { - expect(pollRequest).toHaveBeenCalledTimes(2); - expect(pollStop).toHaveBeenCalledTimes(0); - jest.advanceTimersByTime(pollInterval); - }) - .then(() => waitForPromises()) - .then(() => { - expect(pollRequest).toHaveBeenCalledTimes(MAX_REQUESTS); - expect(pollStop).toHaveBeenCalledTimes(0); - jest.advanceTimersByTime(pollInterval); - }) - .then(() => waitForPromises()) - .then(() => { - expect(pollRequest).toHaveBeenCalledTimes(MAX_REQUESTS + 1); - // Stops poll once it exceeds the MAX_REQUESTS limit - expect(pollStop).toHaveBeenCalledTimes(1); - jest.advanceTimersByTime(pollInterval); - }) - .then(() => waitForPromises()) - .then(() => { - // Additional poll requests are not made once pollStop is called - expect(pollRequest).toHaveBeenCalledTimes(MAX_REQUESTS + 1); - expect(pollStop).toHaveBeenCalledTimes(1); - }) - .then(done) - .catch(done.fail); - }, - ); + }) + .then(() => waitForPromises()) + .then(() => { + expect(pollRequest).toHaveBeenCalledTimes(MAX_REQUESTS); + expect(pollStop).toHaveBeenCalledTimes(0); + jest.advanceTimersByTime(pollInterval); + }) + .then(() => waitForPromises()) + .then(() => { + expect(pollRequest).toHaveBeenCalledTimes(MAX_REQUESTS + 1); + // Stops poll once it exceeds the MAX_REQUESTS limit + expect(pollStop).toHaveBeenCalledTimes(1); + jest.advanceTimersByTime(pollInterval); + }) + .then(() => waitForPromises()) + .then(() => { + // Additional poll requests are not made once pollStop is called + expect(pollRequest).toHaveBeenCalledTimes(MAX_REQUESTS + 1); + expect(pollStop).toHaveBeenCalledTimes(1); + }); }); - it('should stop polling and report to Sentry when data is invalid', (done) => { + it('should stop polling and report to Sentry when data is invalid', async () => { const badApiResponse = { clusters: {} }; mock.onGet().reply(200, badApiResponse, pollHeaders); - testAction( + await testAction( actions.fetchClusters, { endpoint: apiData.endpoint }, {}, @@ -202,12 +192,9 @@ describe('Clusters store actions', () => { }, }, ], - () => { - expect(pollRequest).toHaveBeenCalledTimes(1); - expect(pollStop).toHaveBeenCalledTimes(1); - done(); - }, ); + expect(pollRequest).toHaveBeenCalledTimes(1); + expect(pollStop).toHaveBeenCalledTimes(1); }); }); }); diff --git a/spec/frontend/code_navigation/store/actions_spec.js b/spec/frontend/code_navigation/store/actions_spec.js index af4794eb023..c26416aca94 100644 --- a/spec/frontend/code_navigation/store/actions_spec.js +++ b/spec/frontend/code_navigation/store/actions_spec.js @@ -10,14 +10,13 @@ describe('Code navigation actions', () => { const wrapTextNodes = true; describe('setInitialData', () => { - it('commits SET_INITIAL_DATA', (done) => { - testAction( + it('commits SET_INITIAL_DATA', () => { + return testAction( actions.setInitialData, { projectPath: 'test', wrapTextNodes }, {}, [{ type: 'SET_INITIAL_DATA', payload: { projectPath: 'test', wrapTextNodes } }], [], - done, ); }); }); @@ -59,8 +58,8 @@ describe('Code navigation actions', () => { ]); }); - it('commits REQUEST_DATA_SUCCESS with normalized data', (done) => { - testAction( + it('commits REQUEST_DATA_SUCCESS with normalized data', () => { + return testAction( actions.fetchData, null, state, @@ -82,12 +81,11 @@ describe('Code navigation actions', () => { }, ], [], - done, ); }); - it('calls addInteractionClass with data', (done) => { - testAction( + it('calls addInteractionClass with data', () => { + return testAction( actions.fetchData, null, state, @@ -109,20 +107,17 @@ describe('Code navigation actions', () => { }, ], [], - ) - .then(() => { - expect(addInteractionClass).toHaveBeenCalledWith({ - path: 'index.js', - d: { - start_line: 0, - start_char: 0, - hover: { value: '123' }, - }, - wrapTextNodes, - }); - }) - .then(done) - .catch(done.fail); + ).then(() => { + expect(addInteractionClass).toHaveBeenCalledWith({ + path: 'index.js', + d: { + start_line: 0, + start_char: 0, + hover: { value: '123' }, + }, + wrapTextNodes, + }); + }); }); }); @@ -131,14 +126,13 @@ describe('Code navigation actions', () => { mock.onGet(codeNavigationPath).replyOnce(500); }); - it('dispatches requestDataError', (done) => { - testAction( + it('dispatches requestDataError', () => { + return testAction( actions.fetchData, null, state, [{ type: 'REQUEST_DATA' }], [{ type: 'requestDataError' }], - done, ); }); }); @@ -186,20 +180,20 @@ describe('Code navigation actions', () => { target = document.querySelector('.js-test'); }); - it('returns early when no data exists', (done) => { - testAction(actions.showDefinition, { target }, {}, [], [], done); + it('returns early when no data exists', () => { + return testAction(actions.showDefinition, { target }, {}, [], []); }); - it('commits SET_CURRENT_DEFINITION when target is not code navitation element', (done) => { - testAction(actions.showDefinition, { target }, { data: {} }, [], [], done); + it('commits SET_CURRENT_DEFINITION when target is not code navitation element', () => { + return testAction(actions.showDefinition, { target }, { data: {} }, [], []); }); - it('commits SET_CURRENT_DEFINITION with LSIF data', (done) => { + it('commits SET_CURRENT_DEFINITION with LSIF data', () => { target.classList.add('js-code-navigation'); target.setAttribute('data-line-index', '0'); target.setAttribute('data-char-index', '0'); - testAction( + return testAction( actions.showDefinition, { target }, { data: { 'index.js': { '0:0': { hover: 'test' } } } }, @@ -214,7 +208,6 @@ describe('Code navigation actions', () => { }, ], [], - done, ); }); diff --git a/spec/frontend/commit/pipelines/pipelines_table_spec.js b/spec/frontend/commit/pipelines/pipelines_table_spec.js index 203a4d23160..9b01af1e585 100644 --- a/spec/frontend/commit/pipelines/pipelines_table_spec.js +++ b/spec/frontend/commit/pipelines/pipelines_table_spec.js @@ -120,18 +120,20 @@ describe('Pipelines table in Commits and Merge requests', () => { }); describe('pipeline badge counts', () => { - it('should receive update-pipelines-count event', (done) => { + it('should receive update-pipelines-count event', () => { const element = document.createElement('div'); document.body.appendChild(element); - element.addEventListener('update-pipelines-count', (event) => { - expect(event.detail.pipelineCount).toEqual(10); - done(); + return new Promise((resolve) => { + element.addEventListener('update-pipelines-count', (event) => { + expect(event.detail.pipelineCount).toEqual(10); + resolve(); + }); + + createComponent(); + + element.appendChild(wrapper.vm.$el); }); - - createComponent(); - - element.appendChild(wrapper.vm.$el); }); }); }); diff --git a/spec/frontend/content_editor/extensions/attachment_spec.js b/spec/frontend/content_editor/extensions/attachment_spec.js index 9f035810b9e..d3c42104e47 100644 --- a/spec/frontend/content_editor/extensions/attachment_spec.js +++ b/spec/frontend/content_editor/extensions/attachment_spec.js @@ -200,13 +200,15 @@ describe('content_editor/extensions/attachment', () => { }); }); - it('emits an alert event that includes an error message', (done) => { + it('emits an alert event that includes an error message', () => { tiptapEditor.commands.uploadAttachment({ file }); - eventHub.$on('alert', ({ message, variant }) => { - expect(variant).toBe(VARIANT_DANGER); - expect(message).toBe('An error occurred while uploading the file. Please try again.'); - done(); + return new Promise((resolve) => { + eventHub.$on('alert', ({ message, variant }) => { + expect(variant).toBe(VARIANT_DANGER); + expect(message).toBe('An error occurred while uploading the file. Please try again.'); + resolve(); + }); }); }); }); @@ -277,13 +279,12 @@ describe('content_editor/extensions/attachment', () => { }); }); - it('emits an alert event that includes an error message', (done) => { + it('emits an alert event that includes an error message', () => { tiptapEditor.commands.uploadAttachment({ file: attachmentFile }); eventHub.$on('alert', ({ message, variant }) => { expect(variant).toBe(VARIANT_DANGER); expect(message).toBe('An error occurred while uploading the file. Please try again.'); - done(); }); }); }); diff --git a/spec/frontend/contributors/store/actions_spec.js b/spec/frontend/contributors/store/actions_spec.js index a4054ab1fc8..ef0ff8ca208 100644 --- a/spec/frontend/contributors/store/actions_spec.js +++ b/spec/frontend/contributors/store/actions_spec.js @@ -17,10 +17,14 @@ describe('Contributors store actions', () => { mock = new MockAdapter(axios); }); - it('should commit SET_CHART_DATA with received response', (done) => { + afterEach(() => { + mock.restore(); + }); + + it('should commit SET_CHART_DATA with received response', () => { mock.onGet().reply(200, chartData); - testAction( + return testAction( actions.fetchChartData, { endpoint }, {}, @@ -30,30 +34,22 @@ describe('Contributors store actions', () => { { type: types.SET_LOADING_STATE, payload: false }, ], [], - () => { - mock.restore(); - done(); - }, ); }); - it('should show flash on API error', (done) => { + it('should show flash on API error', async () => { mock.onGet().reply(400, 'Not Found'); - testAction( + await testAction( actions.fetchChartData, { endpoint }, {}, [{ type: types.SET_LOADING_STATE, payload: true }], [], - () => { - expect(createFlash).toHaveBeenCalledWith({ - message: expect.stringMatching('error'), - }); - mock.restore(); - done(); - }, ); + expect(createFlash).toHaveBeenCalledWith({ + message: expect.stringMatching('error'), + }); }); }); }); diff --git a/spec/frontend/create_cluster/gke_cluster/stores/actions_spec.js b/spec/frontend/create_cluster/gke_cluster/stores/actions_spec.js index 55c502b96bb..c365cb6a9f4 100644 --- a/spec/frontend/create_cluster/gke_cluster/stores/actions_spec.js +++ b/spec/frontend/create_cluster/gke_cluster/stores/actions_spec.js @@ -14,53 +14,49 @@ import { describe('GCP Cluster Dropdown Store Actions', () => { describe('setProject', () => { - it('should set project', (done) => { - testAction( + it('should set project', () => { + return testAction( actions.setProject, selectedProjectMock, { selectedProject: {} }, [{ type: 'SET_PROJECT', payload: selectedProjectMock }], [], - done, ); }); }); describe('setZone', () => { - it('should set zone', (done) => { - testAction( + it('should set zone', () => { + return testAction( actions.setZone, selectedZoneMock, { selectedZone: '' }, [{ type: 'SET_ZONE', payload: selectedZoneMock }], [], - done, ); }); }); describe('setMachineType', () => { - it('should set machine type', (done) => { - testAction( + it('should set machine type', () => { + return testAction( actions.setMachineType, selectedMachineTypeMock, { selectedMachineType: '' }, [{ type: 'SET_MACHINE_TYPE', payload: selectedMachineTypeMock }], [], - done, ); }); }); describe('setIsValidatingProjectBilling', () => { - it('should set machine type', (done) => { - testAction( + it('should set machine type', () => { + return testAction( actions.setIsValidatingProjectBilling, true, { isValidatingProjectBilling: null }, [{ type: 'SET_IS_VALIDATING_PROJECT_BILLING', payload: true }], [], - done, ); }); }); @@ -94,8 +90,8 @@ describe('GCP Cluster Dropdown Store Actions', () => { }); describe('validateProjectBilling', () => { - it('checks project billing status from Google API', (done) => { - testAction( + it('checks project billing status from Google API', () => { + return testAction( actions.validateProjectBilling, true, { @@ -110,7 +106,6 @@ describe('GCP Cluster Dropdown Store Actions', () => { { type: 'SET_PROJECT_BILLING_STATUS', payload: true }, ], [{ type: 'setIsValidatingProjectBilling', payload: false }], - done, ); }); }); diff --git a/spec/frontend/diffs/components/diff_line_note_form_spec.js b/spec/frontend/diffs/components/diff_line_note_form_spec.js index 129c0316cc0..fb9dc22ce25 100644 --- a/spec/frontend/diffs/components/diff_line_note_form_spec.js +++ b/spec/frontend/diffs/components/diff_line_note_form_spec.js @@ -98,7 +98,7 @@ describe('DiffLineNoteForm', () => { }); describe('saveNoteForm', () => { - it('should call saveNote action with proper params', (done) => { + it('should call saveNote action with proper params', async () => { const saveDiffDiscussionSpy = jest .spyOn(wrapper.vm, 'saveDiffDiscussion') .mockReturnValue(Promise.resolve()); @@ -123,16 +123,11 @@ describe('DiffLineNoteForm', () => { lineRange, }; - wrapper.vm - .handleSaveNote('note body') - .then(() => { - expect(saveDiffDiscussionSpy).toHaveBeenCalledWith({ - note: 'note body', - formData, - }); - }) - .then(done) - .catch(done.fail); + await wrapper.vm.handleSaveNote('note body'); + expect(saveDiffDiscussionSpy).toHaveBeenCalledWith({ + note: 'note body', + formData, + }); }); }); }); diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index d6a2aa104cd..3b567fbc704 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -9,46 +9,7 @@ import { INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE, } from '~/diffs/constants'; -import { - setBaseConfig, - fetchDiffFilesBatch, - fetchDiffFilesMeta, - fetchCoverageFiles, - assignDiscussionsToDiff, - removeDiscussionsFromDiff, - startRenderDiffsQueue, - setInlineDiffViewType, - setParallelDiffViewType, - showCommentForm, - cancelCommentForm, - loadMoreLines, - scrollToLineIfNeededInline, - scrollToLineIfNeededParallel, - loadCollapsedDiff, - toggleFileDiscussions, - saveDiffDiscussion, - setHighlightedRow, - toggleTreeOpen, - scrollToFile, - setShowTreeList, - renderFileForDiscussionId, - setRenderTreeList, - setShowWhitespace, - setRenderIt, - receiveFullDiffError, - fetchFullDiff, - toggleFullDiff, - switchToFullDiffFromRenamedFile, - setFileCollapsedByUser, - setExpandedDiffLines, - setSuggestPopoverDismissed, - changeCurrentCommit, - moveToNeighboringCommit, - setCurrentDiffFileIdFromNote, - navigateToDiffFileIndex, - setFileByFile, - reviewFile, -} from '~/diffs/store/actions'; +import * as diffActions from '~/diffs/store/actions'; import * as types from '~/diffs/store/mutation_types'; import * as utils from '~/diffs/store/utils'; import * as treeWorkerUtils from '~/diffs/utils/tree_worker_utils'; @@ -62,6 +23,8 @@ import { diffMetadata } from '../mock_data/diff_metadata'; jest.mock('~/flash'); describe('DiffsStoreActions', () => { + let mock; + useLocalStorageSpy(); const originalMethods = { @@ -83,15 +46,20 @@ describe('DiffsStoreActions', () => { }); }); + beforeEach(() => { + mock = new MockAdapter(axios); + }); + afterEach(() => { ['requestAnimationFrame', 'requestIdleCallback'].forEach((method) => { global[method] = originalMethods[method]; }); createFlash.mockClear(); + mock.restore(); }); describe('setBaseConfig', () => { - it('should set given endpoint and project path', (done) => { + it('should set given endpoint and project path', () => { const endpoint = '/diffs/set/endpoint'; const endpointMetadata = '/diffs/set/endpoint/metadata'; const endpointBatch = '/diffs/set/endpoint/batch'; @@ -104,8 +72,8 @@ describe('DiffsStoreActions', () => { b: ['y', 'hash:a'], }; - testAction( - setBaseConfig, + return testAction( + diffActions.setBaseConfig, { endpoint, endpointBatch, @@ -153,23 +121,12 @@ describe('DiffsStoreActions', () => { }, ], [], - done, ); }); }); describe('fetchDiffFilesBatch', () => { - let mock; - - beforeEach(() => { - mock = new MockAdapter(axios); - }); - - afterEach(() => { - mock.restore(); - }); - - it('should fetch batch diff files', (done) => { + it('should fetch batch diff files', () => { const endpointBatch = '/fetch/diffs_batch'; const res1 = { diff_files: [{ file_hash: 'test' }], pagination: { total_pages: 7 } }; const res2 = { diff_files: [{ file_hash: 'test2' }], pagination: { total_pages: 7 } }; @@ -199,8 +156,8 @@ describe('DiffsStoreActions', () => { ) .reply(200, res2); - testAction( - fetchDiffFilesBatch, + return testAction( + diffActions.fetchDiffFilesBatch, {}, { endpointBatch, diffViewType: 'inline', diffFiles: [] }, [ @@ -216,7 +173,6 @@ describe('DiffsStoreActions', () => { { type: types.SET_BATCH_LOADING_STATE, payload: 'error' }, ], [{ type: 'startRenderDiffsQueue' }, { type: 'startRenderDiffsQueue' }], - done, ); }); @@ -229,13 +185,14 @@ describe('DiffsStoreActions', () => { ({ viewStyle, otherView }) => { const endpointBatch = '/fetch/diffs_batch'; - fetchDiffFilesBatch({ - commit: () => {}, - state: { - endpointBatch: `${endpointBatch}?view=${otherView}`, - diffViewType: viewStyle, - }, - }) + diffActions + .fetchDiffFilesBatch({ + commit: () => {}, + state: { + endpointBatch: `${endpointBatch}?view=${otherView}`, + diffViewType: viewStyle, + }, + }) .then(() => { expect(mock.history.get[0].url).toContain(`view=${viewStyle}`); expect(mock.history.get[0].url).not.toContain(`view=${otherView}`); @@ -248,19 +205,16 @@ describe('DiffsStoreActions', () => { describe('fetchDiffFilesMeta', () => { const endpointMetadata = '/fetch/diffs_metadata.json?view=inline'; const noFilesData = { ...diffMetadata }; - let mock; beforeEach(() => { - mock = new MockAdapter(axios); - delete noFilesData.diff_files; mock.onGet(endpointMetadata).reply(200, diffMetadata); }); - it('should fetch diff meta information', (done) => { - testAction( - fetchDiffFilesMeta, + it('should fetch diff meta information', () => { + return testAction( + diffActions.fetchDiffFilesMeta, {}, { endpointMetadata, diffViewType: 'inline' }, [ @@ -275,55 +229,41 @@ describe('DiffsStoreActions', () => { }, ], [], - () => { - mock.restore(); - done(); - }, ); }); }); describe('fetchCoverageFiles', () => { - let mock; const endpointCoverage = '/fetch'; - beforeEach(() => { - mock = new MockAdapter(axios); - }); - - afterEach(() => mock.restore()); - - it('should commit SET_COVERAGE_DATA with received response', (done) => { + it('should commit SET_COVERAGE_DATA with received response', () => { const data = { files: { 'app.js': { 1: 0, 2: 1 } } }; mock.onGet(endpointCoverage).reply(200, { data }); - testAction( - fetchCoverageFiles, + return testAction( + diffActions.fetchCoverageFiles, {}, { endpointCoverage }, [{ type: types.SET_COVERAGE_DATA, payload: { data } }], [], - done, ); }); - it('should show flash on API error', (done) => { + it('should show flash on API error', async () => { mock.onGet(endpointCoverage).reply(400); - testAction(fetchCoverageFiles, {}, { endpointCoverage }, [], [], () => { - expect(createFlash).toHaveBeenCalledTimes(1); - expect(createFlash).toHaveBeenCalledWith({ - message: expect.stringMatching('Something went wrong'), - }); - done(); + await testAction(diffActions.fetchCoverageFiles, {}, { endpointCoverage }, [], []); + expect(createFlash).toHaveBeenCalledTimes(1); + expect(createFlash).toHaveBeenCalledWith({ + message: expect.stringMatching('Something went wrong'), }); }); }); describe('setHighlightedRow', () => { it('should mark currently selected diff and set lineHash and fileHash of highlightedRow', () => { - testAction(setHighlightedRow, 'ABC_123', {}, [ + return testAction(diffActions.setHighlightedRow, 'ABC_123', {}, [ { type: types.SET_HIGHLIGHTED_ROW, payload: 'ABC_123' }, { type: types.SET_CURRENT_DIFF_FILE, payload: 'ABC' }, ]); @@ -335,7 +275,7 @@ describe('DiffsStoreActions', () => { window.location.hash = ''; }); - it('should merge discussions into diffs', (done) => { + it('should merge discussions into diffs', () => { window.location.hash = 'ABC_123'; const state = { @@ -397,8 +337,8 @@ describe('DiffsStoreActions', () => { const discussions = [singleDiscussion]; - testAction( - assignDiscussionsToDiff, + return testAction( + diffActions.assignDiscussionsToDiff, discussions, state, [ @@ -425,26 +365,24 @@ describe('DiffsStoreActions', () => { }, ], [], - done, ); }); - it('dispatches setCurrentDiffFileIdFromNote with note ID', (done) => { + it('dispatches setCurrentDiffFileIdFromNote with note ID', () => { window.location.hash = 'note_123'; - testAction( - assignDiscussionsToDiff, + return testAction( + diffActions.assignDiscussionsToDiff, [], { diffFiles: [] }, [], [{ type: 'setCurrentDiffFileIdFromNote', payload: '123' }], - done, ); }); }); describe('removeDiscussionsFromDiff', () => { - it('should remove discussions from diffs', (done) => { + it('should remove discussions from diffs', () => { const state = { diffFiles: [ { @@ -480,8 +418,8 @@ describe('DiffsStoreActions', () => { line_code: 'ABC_1_1', }; - testAction( - removeDiscussionsFromDiff, + return testAction( + diffActions.removeDiscussionsFromDiff, singleDiscussion, state, [ @@ -495,7 +433,6 @@ describe('DiffsStoreActions', () => { }, ], [], - done, ); }); }); @@ -528,7 +465,7 @@ describe('DiffsStoreActions', () => { }); }; - startRenderDiffsQueue({ state, commit: pseudoCommit }); + diffActions.startRenderDiffsQueue({ state, commit: pseudoCommit }); expect(state.diffFiles[0].renderIt).toBe(true); expect(state.diffFiles[1].renderIt).toBe(true); @@ -536,69 +473,61 @@ describe('DiffsStoreActions', () => { }); describe('setInlineDiffViewType', () => { - it('should set diff view type to inline and also set the cookie properly', (done) => { - testAction( - setInlineDiffViewType, + it('should set diff view type to inline and also set the cookie properly', async () => { + await testAction( + diffActions.setInlineDiffViewType, null, {}, [{ type: types.SET_DIFF_VIEW_TYPE, payload: INLINE_DIFF_VIEW_TYPE }], [], - () => { - expect(Cookies.get('diff_view')).toEqual(INLINE_DIFF_VIEW_TYPE); - done(); - }, ); + expect(Cookies.get('diff_view')).toEqual(INLINE_DIFF_VIEW_TYPE); }); }); describe('setParallelDiffViewType', () => { - it('should set diff view type to parallel and also set the cookie properly', (done) => { - testAction( - setParallelDiffViewType, + it('should set diff view type to parallel and also set the cookie properly', async () => { + await testAction( + diffActions.setParallelDiffViewType, null, {}, [{ type: types.SET_DIFF_VIEW_TYPE, payload: PARALLEL_DIFF_VIEW_TYPE }], [], - () => { - expect(Cookies.get(DIFF_VIEW_COOKIE_NAME)).toEqual(PARALLEL_DIFF_VIEW_TYPE); - done(); - }, ); + expect(Cookies.get(DIFF_VIEW_COOKIE_NAME)).toEqual(PARALLEL_DIFF_VIEW_TYPE); }); }); describe('showCommentForm', () => { - it('should call mutation to show comment form', (done) => { + it('should call mutation to show comment form', () => { const payload = { lineCode: 'lineCode', fileHash: 'hash' }; - testAction( - showCommentForm, + return testAction( + diffActions.showCommentForm, payload, {}, [{ type: types.TOGGLE_LINE_HAS_FORM, payload: { ...payload, hasForm: true } }], [], - done, ); }); }); describe('cancelCommentForm', () => { - it('should call mutation to cancel comment form', (done) => { + it('should call mutation to cancel comment form', () => { const payload = { lineCode: 'lineCode', fileHash: 'hash' }; - testAction( - cancelCommentForm, + return testAction( + diffActions.cancelCommentForm, payload, {}, [{ type: types.TOGGLE_LINE_HAS_FORM, payload: { ...payload, hasForm: false } }], [], - done, ); }); }); describe('loadMoreLines', () => { - it('should call mutation to show comment form', (done) => { + it('should call mutation to show comment form', () => { const endpoint = '/diffs/load/more/lines'; const params = { since: 6, to: 26 }; const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; @@ -606,12 +535,11 @@ describe('DiffsStoreActions', () => { const isExpandDown = false; const nextLineNumbers = {}; const options = { endpoint, params, lineNumbers, fileHash, isExpandDown, nextLineNumbers }; - const mock = new MockAdapter(axios); const contextLines = { contextLines: [{ lineCode: 6 }] }; mock.onGet(endpoint).reply(200, contextLines); - testAction( - loadMoreLines, + return testAction( + diffActions.loadMoreLines, options, {}, [ @@ -621,31 +549,23 @@ describe('DiffsStoreActions', () => { }, ], [], - () => { - mock.restore(); - done(); - }, ); }); }); describe('loadCollapsedDiff', () => { const state = { showWhitespace: true }; - it('should fetch data and call mutation with response and the give parameter', (done) => { + it('should fetch data and call mutation with response and the give parameter', () => { const file = { hash: 123, load_collapsed_diff_url: '/load/collapsed/diff/url' }; const data = { hash: 123, parallelDiffLines: [{ lineCode: 1 }] }; - const mock = new MockAdapter(axios); const commit = jest.fn(); mock.onGet(file.loadCollapsedDiffUrl).reply(200, data); - loadCollapsedDiff({ commit, getters: { commitId: null }, state }, file) + return diffActions + .loadCollapsedDiff({ commit, getters: { commitId: null }, state }, file) .then(() => { expect(commit).toHaveBeenCalledWith(types.ADD_COLLAPSED_DIFFS, { file, data }); - - mock.restore(); - done(); - }) - .catch(done.fail); + }); }); it('should fetch data without commit ID', () => { @@ -656,7 +576,7 @@ describe('DiffsStoreActions', () => { jest.spyOn(axios, 'get').mockReturnValue(Promise.resolve({ data: {} })); - loadCollapsedDiff({ commit() {}, getters, state }, file); + diffActions.loadCollapsedDiff({ commit() {}, getters, state }, file); expect(axios.get).toHaveBeenCalledWith(file.load_collapsed_diff_url, { params: { commit_id: null, w: '0' }, @@ -671,7 +591,7 @@ describe('DiffsStoreActions', () => { jest.spyOn(axios, 'get').mockReturnValue(Promise.resolve({ data: {} })); - loadCollapsedDiff({ commit() {}, getters, state }, file); + diffActions.loadCollapsedDiff({ commit() {}, getters, state }, file); expect(axios.get).toHaveBeenCalledWith(file.load_collapsed_diff_url, { params: { commit_id: '123', w: '0' }, @@ -689,7 +609,7 @@ describe('DiffsStoreActions', () => { const dispatch = jest.fn(); - toggleFileDiscussions({ getters, dispatch }); + diffActions.toggleFileDiscussions({ getters, dispatch }); expect(dispatch).toHaveBeenCalledWith( 'collapseDiscussion', @@ -707,7 +627,7 @@ describe('DiffsStoreActions', () => { const dispatch = jest.fn(); - toggleFileDiscussions({ getters, dispatch }); + diffActions.toggleFileDiscussions({ getters, dispatch }); expect(dispatch).toHaveBeenCalledWith( 'expandDiscussion', @@ -725,7 +645,7 @@ describe('DiffsStoreActions', () => { const dispatch = jest.fn(); - toggleFileDiscussions({ getters, dispatch }); + diffActions.toggleFileDiscussions({ getters, dispatch }); expect(dispatch).toHaveBeenCalledWith( 'expandDiscussion', @@ -743,7 +663,7 @@ describe('DiffsStoreActions', () => { it('should not call handleLocationHash when there is not hash', () => { window.location.hash = ''; - scrollToLineIfNeededInline({}, lineMock); + diffActions.scrollToLineIfNeededInline({}, lineMock); expect(commonUtils.handleLocationHash).not.toHaveBeenCalled(); }); @@ -751,7 +671,7 @@ describe('DiffsStoreActions', () => { it('should not call handleLocationHash when the hash does not match any line', () => { window.location.hash = 'XYZ_456'; - scrollToLineIfNeededInline({}, lineMock); + diffActions.scrollToLineIfNeededInline({}, lineMock); expect(commonUtils.handleLocationHash).not.toHaveBeenCalled(); }); @@ -759,14 +679,14 @@ describe('DiffsStoreActions', () => { it('should call handleLocationHash only when the hash matches a line', () => { window.location.hash = 'ABC_123'; - scrollToLineIfNeededInline( + diffActions.scrollToLineIfNeededInline( {}, { lineCode: 'ABC_456', }, ); - scrollToLineIfNeededInline({}, lineMock); - scrollToLineIfNeededInline( + diffActions.scrollToLineIfNeededInline({}, lineMock); + diffActions.scrollToLineIfNeededInline( {}, { lineCode: 'XYZ_456', @@ -789,7 +709,7 @@ describe('DiffsStoreActions', () => { it('should not call handleLocationHash when there is not hash', () => { window.location.hash = ''; - scrollToLineIfNeededParallel({}, lineMock); + diffActions.scrollToLineIfNeededParallel({}, lineMock); expect(commonUtils.handleLocationHash).not.toHaveBeenCalled(); }); @@ -797,7 +717,7 @@ describe('DiffsStoreActions', () => { it('should not call handleLocationHash when the hash does not match any line', () => { window.location.hash = 'XYZ_456'; - scrollToLineIfNeededParallel({}, lineMock); + diffActions.scrollToLineIfNeededParallel({}, lineMock); expect(commonUtils.handleLocationHash).not.toHaveBeenCalled(); }); @@ -805,7 +725,7 @@ describe('DiffsStoreActions', () => { it('should call handleLocationHash only when the hash matches a line', () => { window.location.hash = 'ABC_123'; - scrollToLineIfNeededParallel( + diffActions.scrollToLineIfNeededParallel( {}, { left: null, @@ -814,8 +734,8 @@ describe('DiffsStoreActions', () => { }, }, ); - scrollToLineIfNeededParallel({}, lineMock); - scrollToLineIfNeededParallel( + diffActions.scrollToLineIfNeededParallel({}, lineMock); + diffActions.scrollToLineIfNeededParallel( {}, { left: null, @@ -831,7 +751,7 @@ describe('DiffsStoreActions', () => { }); describe('saveDiffDiscussion', () => { - it('dispatches actions', (done) => { + it('dispatches actions', () => { const commitId = 'something'; const formData = { diffFile: { ...mockDiffFile }, @@ -856,33 +776,29 @@ describe('DiffsStoreActions', () => { } }); - saveDiffDiscussion({ state, dispatch }, { note, formData }) - .then(() => { - expect(dispatch).toHaveBeenCalledTimes(5); - expect(dispatch).toHaveBeenNthCalledWith(1, 'saveNote', expect.any(Object), { - root: true, - }); + return diffActions.saveDiffDiscussion({ state, dispatch }, { note, formData }).then(() => { + expect(dispatch).toHaveBeenCalledTimes(5); + expect(dispatch).toHaveBeenNthCalledWith(1, 'saveNote', expect.any(Object), { + root: true, + }); - const postData = dispatch.mock.calls[0][1]; - expect(postData.data.note.commit_id).toBe(commitId); + const postData = dispatch.mock.calls[0][1]; + expect(postData.data.note.commit_id).toBe(commitId); - expect(dispatch).toHaveBeenNthCalledWith(2, 'updateDiscussion', 'test', { root: true }); - expect(dispatch).toHaveBeenNthCalledWith(3, 'assignDiscussionsToDiff', ['discussion']); - }) - .then(done) - .catch(done.fail); + expect(dispatch).toHaveBeenNthCalledWith(2, 'updateDiscussion', 'test', { root: true }); + expect(dispatch).toHaveBeenNthCalledWith(3, 'assignDiscussionsToDiff', ['discussion']); + }); }); }); describe('toggleTreeOpen', () => { - it('commits TOGGLE_FOLDER_OPEN', (done) => { - testAction( - toggleTreeOpen, + it('commits TOGGLE_FOLDER_OPEN', () => { + return testAction( + diffActions.toggleTreeOpen, 'path', {}, [{ type: types.TOGGLE_FOLDER_OPEN, payload: 'path' }], [], - done, ); }); }); @@ -904,7 +820,7 @@ describe('DiffsStoreActions', () => { }, }; - scrollToFile({ state, commit, getters }, { path: 'path' }); + diffActions.scrollToFile({ state, commit, getters }, { path: 'path' }); expect(document.location.hash).toBe('#test'); }); @@ -918,28 +834,27 @@ describe('DiffsStoreActions', () => { }, }; - scrollToFile({ state, commit, getters }, { path: 'path' }); + diffActions.scrollToFile({ state, commit, getters }, { path: 'path' }); expect(commit).toHaveBeenCalledWith(types.SET_CURRENT_DIFF_FILE, 'test'); }); }); describe('setShowTreeList', () => { - it('commits toggle', (done) => { - testAction( - setShowTreeList, + it('commits toggle', () => { + return testAction( + diffActions.setShowTreeList, { showTreeList: true }, {}, [{ type: types.SET_SHOW_TREE_LIST, payload: true }], [], - done, ); }); it('updates localStorage', () => { jest.spyOn(localStorage, 'setItem').mockImplementation(() => {}); - setShowTreeList({ commit() {} }, { showTreeList: true }); + diffActions.setShowTreeList({ commit() {} }, { showTreeList: true }); expect(localStorage.setItem).toHaveBeenCalledWith('mr_tree_show', true); }); @@ -947,7 +862,7 @@ describe('DiffsStoreActions', () => { it('does not update localStorage', () => { jest.spyOn(localStorage, 'setItem').mockImplementation(() => {}); - setShowTreeList({ commit() {} }, { showTreeList: true, saving: false }); + diffActions.setShowTreeList({ commit() {} }, { showTreeList: true, saving: false }); expect(localStorage.setItem).not.toHaveBeenCalled(); }); @@ -994,7 +909,7 @@ describe('DiffsStoreActions', () => { it('renders and expands file for the given discussion id', () => { const localState = state({ collapsed: true, renderIt: false }); - renderFileForDiscussionId({ rootState, state: localState, commit }, '123'); + diffActions.renderFileForDiscussionId({ rootState, state: localState, commit }, '123'); expect(commit).toHaveBeenCalledWith('RENDER_FILE', localState.diffFiles[0]); expect($emit).toHaveBeenCalledTimes(1); @@ -1004,7 +919,7 @@ describe('DiffsStoreActions', () => { it('jumps to discussion on already rendered and expanded file', () => { const localState = state({ collapsed: false, renderIt: true }); - renderFileForDiscussionId({ rootState, state: localState, commit }, '123'); + diffActions.renderFileForDiscussionId({ rootState, state: localState, commit }, '123'); expect(commit).not.toHaveBeenCalled(); expect($emit).toHaveBeenCalledTimes(1); @@ -1013,19 +928,18 @@ describe('DiffsStoreActions', () => { }); describe('setRenderTreeList', () => { - it('commits SET_RENDER_TREE_LIST', (done) => { - testAction( - setRenderTreeList, + it('commits SET_RENDER_TREE_LIST', () => { + return testAction( + diffActions.setRenderTreeList, { renderTreeList: true }, {}, [{ type: types.SET_RENDER_TREE_LIST, payload: true }], [], - done, ); }); it('sets localStorage', () => { - setRenderTreeList({ commit() {} }, { renderTreeList: true }); + diffActions.setRenderTreeList({ commit() {} }, { renderTreeList: true }); expect(localStorage.setItem).toHaveBeenCalledWith('mr_diff_tree_list', true); }); @@ -1034,11 +948,9 @@ describe('DiffsStoreActions', () => { describe('setShowWhitespace', () => { const endpointUpdateUser = 'user/prefs'; let putSpy; - let mock; let gon; beforeEach(() => { - mock = new MockAdapter(axios); putSpy = jest.spyOn(axios, 'put'); gon = window.gon; @@ -1047,25 +959,23 @@ describe('DiffsStoreActions', () => { }); afterEach(() => { - mock.restore(); window.gon = gon; }); - it('commits SET_SHOW_WHITESPACE', (done) => { - testAction( - setShowWhitespace, + it('commits SET_SHOW_WHITESPACE', () => { + return testAction( + diffActions.setShowWhitespace, { showWhitespace: true, updateDatabase: false }, {}, [{ type: types.SET_SHOW_WHITESPACE, payload: true }], [], - done, ); }); it('saves to the database when the user is logged in', async () => { window.gon = { current_user_id: 12345 }; - await setShowWhitespace( + await diffActions.setShowWhitespace( { state: { endpointUpdateUser }, commit() {} }, { showWhitespace: true, updateDatabase: true }, ); @@ -1076,7 +986,7 @@ describe('DiffsStoreActions', () => { it('does not try to save to the API if the user is not logged in', async () => { window.gon = {}; - await setShowWhitespace( + await diffActions.setShowWhitespace( { state: { endpointUpdateUser }, commit() {} }, { showWhitespace: true, updateDatabase: true }, ); @@ -1085,7 +995,7 @@ describe('DiffsStoreActions', () => { }); it('emits eventHub event', async () => { - await setShowWhitespace( + await diffActions.setShowWhitespace( { state: {}, commit() {} }, { showWhitespace: true, updateDatabase: false }, ); @@ -1095,53 +1005,47 @@ describe('DiffsStoreActions', () => { }); describe('setRenderIt', () => { - it('commits RENDER_FILE', (done) => { - testAction(setRenderIt, 'file', {}, [{ type: types.RENDER_FILE, payload: 'file' }], [], done); + it('commits RENDER_FILE', () => { + return testAction( + diffActions.setRenderIt, + 'file', + {}, + [{ type: types.RENDER_FILE, payload: 'file' }], + [], + ); }); }); describe('receiveFullDiffError', () => { - it('updates state with the file that did not load', (done) => { - testAction( - receiveFullDiffError, + it('updates state with the file that did not load', () => { + return testAction( + diffActions.receiveFullDiffError, 'file', {}, [{ type: types.RECEIVE_FULL_DIFF_ERROR, payload: 'file' }], [], - done, ); }); }); describe('fetchFullDiff', () => { - let mock; - - beforeEach(() => { - mock = new MockAdapter(axios); - }); - - afterEach(() => { - mock.restore(); - }); - describe('success', () => { beforeEach(() => { mock.onGet(`${TEST_HOST}/context`).replyOnce(200, ['test']); }); - it('commits the success and dispatches an action to expand the new lines', (done) => { + it('commits the success and dispatches an action to expand the new lines', () => { const file = { context_lines_path: `${TEST_HOST}/context`, file_path: 'test', file_hash: 'test', }; - testAction( - fetchFullDiff, + return testAction( + diffActions.fetchFullDiff, file, null, [{ type: types.RECEIVE_FULL_DIFF_SUCCESS, payload: { filePath: 'test' } }], [{ type: 'setExpandedDiffLines', payload: { file, data: ['test'] } }], - done, ); }); }); @@ -1151,14 +1055,13 @@ describe('DiffsStoreActions', () => { mock.onGet(`${TEST_HOST}/context`).replyOnce(500); }); - it('dispatches receiveFullDiffError', (done) => { - testAction( - fetchFullDiff, + it('dispatches receiveFullDiffError', () => { + return testAction( + diffActions.fetchFullDiff, { context_lines_path: `${TEST_HOST}/context`, file_path: 'test', file_hash: 'test' }, null, [], [{ type: 'receiveFullDiffError', payload: 'test' }], - done, ); }); }); @@ -1173,14 +1076,13 @@ describe('DiffsStoreActions', () => { }; }); - it('dispatches fetchFullDiff when file is not expanded', (done) => { - testAction( - toggleFullDiff, + it('dispatches fetchFullDiff when file is not expanded', () => { + return testAction( + diffActions.toggleFullDiff, 'test', state, [{ type: types.REQUEST_FULL_DIFF, payload: 'test' }], [{ type: 'fetchFullDiff', payload: state.diffFiles[0] }], - done, ); }); }); @@ -1202,16 +1104,13 @@ describe('DiffsStoreActions', () => { }; const testData = [{ rich_text: 'test' }, { rich_text: 'file2' }]; let renamedFile; - let mock; beforeEach(() => { - mock = new MockAdapter(axios); jest.spyOn(utils, 'prepareLineForRenamedFile').mockImplementation(() => preparedLine); }); afterEach(() => { renamedFile = null; - mock.restore(); }); describe('success', () => { @@ -1228,7 +1127,7 @@ describe('DiffsStoreActions', () => { 'performs the correct mutations and starts a render queue for view type $diffViewType', ({ diffViewType }) => { return testAction( - switchToFullDiffFromRenamedFile, + diffActions.switchToFullDiffFromRenamedFile, { diffFile: renamedFile }, { diffViewType }, [ @@ -1249,9 +1148,9 @@ describe('DiffsStoreActions', () => { }); describe('setFileUserCollapsed', () => { - it('commits SET_FILE_COLLAPSED', (done) => { - testAction( - setFileCollapsedByUser, + it('commits SET_FILE_COLLAPSED', () => { + return testAction( + diffActions.setFileCollapsedByUser, { filePath: 'test', collapsed: true }, null, [ @@ -1261,7 +1160,6 @@ describe('DiffsStoreActions', () => { }, ], [], - done, ); }); }); @@ -1273,11 +1171,11 @@ describe('DiffsStoreActions', () => { }); }); - it('commits SET_CURRENT_VIEW_DIFF_FILE_LINES when lines less than MAX_RENDERING_DIFF_LINES', (done) => { + it('commits SET_CURRENT_VIEW_DIFF_FILE_LINES when lines less than MAX_RENDERING_DIFF_LINES', () => { utils.convertExpandLines.mockImplementation(() => ['test']); - testAction( - setExpandedDiffLines, + return testAction( + diffActions.setExpandedDiffLines, { file: { file_path: 'path' }, data: [] }, { diffViewType: 'inline' }, [ @@ -1287,16 +1185,15 @@ describe('DiffsStoreActions', () => { }, ], [], - done, ); }); - it('commits ADD_CURRENT_VIEW_DIFF_FILE_LINES when lines more than MAX_RENDERING_DIFF_LINES', (done) => { + it('commits ADD_CURRENT_VIEW_DIFF_FILE_LINES when lines more than MAX_RENDERING_DIFF_LINES', () => { const lines = new Array(501).fill().map((_, i) => `line-${i}`); utils.convertExpandLines.mockReturnValue(lines); - testAction( - setExpandedDiffLines, + return testAction( + diffActions.setExpandedDiffLines, { file: { file_path: 'path' }, data: [] }, { diffViewType: 'inline' }, [ @@ -1312,41 +1209,34 @@ describe('DiffsStoreActions', () => { { type: 'TOGGLE_DIFF_FILE_RENDERING_MORE', payload: 'path' }, ], [], - done, ); }); }); describe('setSuggestPopoverDismissed', () => { - it('commits SET_SHOW_SUGGEST_POPOVER', (done) => { + it('commits SET_SHOW_SUGGEST_POPOVER', async () => { const state = { dismissEndpoint: `${TEST_HOST}/-/user_callouts` }; - const mock = new MockAdapter(axios); mock.onPost(state.dismissEndpoint).reply(200, {}); jest.spyOn(axios, 'post'); - testAction( - setSuggestPopoverDismissed, + await testAction( + diffActions.setSuggestPopoverDismissed, null, state, [{ type: types.SET_SHOW_SUGGEST_POPOVER }], [], - () => { - expect(axios.post).toHaveBeenCalledWith(state.dismissEndpoint, { - feature_name: 'suggest_popover_dismissed', - }); - - mock.restore(); - done(); - }, ); + expect(axios.post).toHaveBeenCalledWith(state.dismissEndpoint, { + feature_name: 'suggest_popover_dismissed', + }); }); }); describe('changeCurrentCommit', () => { it('commits the new commit information and re-requests the diff metadata for the commit', () => { return testAction( - changeCurrentCommit, + diffActions.changeCurrentCommit, { commitId: 'NEW' }, { commit: { @@ -1384,7 +1274,7 @@ describe('DiffsStoreActions', () => { ({ commitId, commit, msg }) => { const err = new Error(msg); const actionReturn = testAction( - changeCurrentCommit, + diffActions.changeCurrentCommit, { commitId }, { endpoint: 'URL/OLD', @@ -1410,7 +1300,7 @@ describe('DiffsStoreActions', () => { 'for the direction "$direction", dispatches the action to move to the SHA "$expected"', ({ direction, expected, currentCommit }) => { return testAction( - moveToNeighboringCommit, + diffActions.moveToNeighboringCommit, { direction }, { commit: currentCommit }, [], @@ -1431,7 +1321,7 @@ describe('DiffsStoreActions', () => { 'given `{ "isloading": $diffsAreLoading, "commit": $currentCommit }` in state, no actions are dispatched', ({ direction, diffsAreLoading, currentCommit }) => { return testAction( - moveToNeighboringCommit, + diffActions.moveToNeighboringCommit, { direction }, { commit: currentCommit, isLoading: diffsAreLoading }, [], @@ -1450,7 +1340,7 @@ describe('DiffsStoreActions', () => { notesById: { 1: { discussion_id: '2' } }, }; - setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); + diffActions.setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); expect(commit).toHaveBeenCalledWith(types.SET_CURRENT_DIFF_FILE, '123'); }); @@ -1463,7 +1353,7 @@ describe('DiffsStoreActions', () => { notesById: { 1: { discussion_id: '2' } }, }; - setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); + diffActions.setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); expect(commit).not.toHaveBeenCalled(); }); @@ -1476,21 +1366,20 @@ describe('DiffsStoreActions', () => { notesById: { 1: { discussion_id: '2' } }, }; - setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); + diffActions.setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); expect(commit).not.toHaveBeenCalled(); }); }); describe('navigateToDiffFileIndex', () => { - it('commits SET_CURRENT_DIFF_FILE', (done) => { - testAction( - navigateToDiffFileIndex, + it('commits SET_CURRENT_DIFF_FILE', () => { + return testAction( + diffActions.navigateToDiffFileIndex, 0, { diffFiles: [{ file_hash: '123' }] }, [{ type: types.SET_CURRENT_DIFF_FILE, payload: '123' }], [], - done, ); }); }); @@ -1498,19 +1387,13 @@ describe('DiffsStoreActions', () => { describe('setFileByFile', () => { const updateUserEndpoint = 'user/prefs'; let putSpy; - let mock; beforeEach(() => { - mock = new MockAdapter(axios); putSpy = jest.spyOn(axios, 'put'); mock.onPut(updateUserEndpoint).reply(200, {}); }); - afterEach(() => { - mock.restore(); - }); - it.each` value ${true} @@ -1519,7 +1402,7 @@ describe('DiffsStoreActions', () => { 'commits SET_FILE_BY_FILE and persists the File-by-File user preference with the new value $value', async ({ value }) => { await testAction( - setFileByFile, + diffActions.setFileByFile, { fileByFile: value }, { viewDiffsFileByFile: null, @@ -1551,7 +1434,7 @@ describe('DiffsStoreActions', () => { const commitSpy = jest.fn(); const getterSpy = jest.fn().mockReturnValue([]); - reviewFile( + diffActions.reviewFile( { commit: commitSpy, getters: { diff --git a/spec/frontend/environments/deploy_board_component_spec.js b/spec/frontend/environments/deploy_board_component_spec.js index f0fb4d1027c..6bf87f7b07f 100644 --- a/spec/frontend/environments/deploy_board_component_spec.js +++ b/spec/frontend/environments/deploy_board_component_spec.js @@ -23,9 +23,9 @@ describe('Deploy Board', () => { }); describe('with valid data', () => { - beforeEach((done) => { + beforeEach(() => { wrapper = createComponent(); - nextTick(done); + return nextTick(); }); it('should render percentage with completion value provided', () => { @@ -127,14 +127,14 @@ describe('Deploy Board', () => { }); describe('with empty state', () => { - beforeEach((done) => { + beforeEach(() => { wrapper = createComponent({ deployBoardData: {}, isLoading: false, isEmpty: true, logsPath, }); - nextTick(done); + return nextTick(); }); it('should render the empty state', () => { @@ -146,14 +146,14 @@ describe('Deploy Board', () => { }); describe('with loading state', () => { - beforeEach((done) => { + beforeEach(() => { wrapper = createComponent({ deployBoardData: {}, isLoading: true, isEmpty: false, logsPath, }); - nextTick(done); + return nextTick(); }); it('should render loading spinner', () => { @@ -163,7 +163,7 @@ describe('Deploy Board', () => { describe('has legend component', () => { let statuses = []; - beforeEach((done) => { + beforeEach(() => { wrapper = createComponent({ isLoading: false, isEmpty: false, @@ -171,7 +171,7 @@ describe('Deploy Board', () => { deployBoardData: deployBoardMockData, }); ({ statuses } = wrapper.vm); - nextTick(done); + return nextTick(); }); it('with all the possible statuses', () => { diff --git a/spec/frontend/environments/environment_table_spec.js b/spec/frontend/environments/environment_table_spec.js index c7582e4b06d..666e87c748e 100644 --- a/spec/frontend/environments/environment_table_spec.js +++ b/spec/frontend/environments/environment_table_spec.js @@ -122,7 +122,7 @@ describe('Environment table', () => { expect(wrapper.find('.deploy-board-icon').exists()).toBe(true); }); - it('should toggle deploy board visibility when arrow is clicked', (done) => { + it('should toggle deploy board visibility when arrow is clicked', async () => { const mockItem = { name: 'review', size: 1, @@ -142,7 +142,6 @@ describe('Environment table', () => { eventHub.$on('toggleDeployBoard', (env) => { expect(env.id).toEqual(mockItem.id); - done(); }); factory({ @@ -154,7 +153,7 @@ describe('Environment table', () => { }, }); - wrapper.find('.deploy-board-icon').trigger('click'); + await wrapper.find('.deploy-board-icon').trigger('click'); }); it('should set the environment to change and weight when a change canary weight event is recevied', async () => { diff --git a/spec/frontend/error_tracking/store/actions_spec.js b/spec/frontend/error_tracking/store/actions_spec.js index aaaa1194a29..6bac21341a7 100644 --- a/spec/frontend/error_tracking/store/actions_spec.js +++ b/spec/frontend/error_tracking/store/actions_spec.js @@ -28,9 +28,9 @@ describe('Sentry common store actions', () => { const params = { endpoint, redirectUrl, status }; describe('updateStatus', () => { - it('should handle successful status update', (done) => { + it('should handle successful status update', async () => { mock.onPut().reply(200, {}); - testAction( + await testAction( actions.updateStatus, params, {}, @@ -41,20 +41,15 @@ describe('Sentry common store actions', () => { }, ], [], - () => { - done(); - expect(visitUrl).toHaveBeenCalledWith(redirectUrl); - }, ); + expect(visitUrl).toHaveBeenCalledWith(redirectUrl); }); - it('should handle unsuccessful status update', (done) => { + it('should handle unsuccessful status update', async () => { mock.onPut().reply(400, {}); - testAction(actions.updateStatus, params, {}, [], [], () => { - expect(visitUrl).not.toHaveBeenCalled(); - expect(createFlash).toHaveBeenCalledTimes(1); - done(); - }); + await testAction(actions.updateStatus, params, {}, [], []); + expect(visitUrl).not.toHaveBeenCalled(); + expect(createFlash).toHaveBeenCalledTimes(1); }); }); diff --git a/spec/frontend/error_tracking/store/details/actions_spec.js b/spec/frontend/error_tracking/store/details/actions_spec.js index 623cb82851d..a3a6f7cc309 100644 --- a/spec/frontend/error_tracking/store/details/actions_spec.js +++ b/spec/frontend/error_tracking/store/details/actions_spec.js @@ -28,10 +28,10 @@ describe('Sentry error details store actions', () => { describe('startPollingStacktrace', () => { const endpoint = '123/stacktrace'; - it('should commit SET_ERROR with received response', (done) => { + it('should commit SET_ERROR with received response', () => { const payload = { error: [1, 2, 3] }; mockedAdapter.onGet().reply(200, payload); - testAction( + return testAction( actions.startPollingStacktrace, { endpoint }, {}, @@ -40,37 +40,29 @@ describe('Sentry error details store actions', () => { { type: types.SET_LOADING_STACKTRACE, payload: false }, ], [], - () => { - done(); - }, ); }); - it('should show flash on API error', (done) => { + it('should show flash on API error', async () => { mockedAdapter.onGet().reply(400); - testAction( + await testAction( actions.startPollingStacktrace, { endpoint }, {}, [{ type: types.SET_LOADING_STACKTRACE, payload: false }], [], - () => { - expect(createFlash).toHaveBeenCalledTimes(1); - done(); - }, ); + expect(createFlash).toHaveBeenCalledTimes(1); }); - it('should not restart polling when receiving an empty 204 response', (done) => { + it('should not restart polling when receiving an empty 204 response', async () => { mockedRestart = jest.spyOn(Poll.prototype, 'restart'); mockedAdapter.onGet().reply(204); - testAction(actions.startPollingStacktrace, { endpoint }, {}, [], [], () => { - mockedRestart = jest.spyOn(Poll.prototype, 'restart'); - expect(mockedRestart).toHaveBeenCalledTimes(0); - done(); - }); + await testAction(actions.startPollingStacktrace, { endpoint }, {}, [], []); + mockedRestart = jest.spyOn(Poll.prototype, 'restart'); + expect(mockedRestart).toHaveBeenCalledTimes(0); }); }); }); diff --git a/spec/frontend/error_tracking/store/list/actions_spec.js b/spec/frontend/error_tracking/store/list/actions_spec.js index 5465bde397c..7173f68bb96 100644 --- a/spec/frontend/error_tracking/store/list/actions_spec.js +++ b/spec/frontend/error_tracking/store/list/actions_spec.js @@ -20,11 +20,11 @@ describe('error tracking actions', () => { }); describe('startPolling', () => { - it('should start polling for data', (done) => { + it('should start polling for data', () => { const payload = { errors: [{ id: 1 }, { id: 2 }] }; mock.onGet().reply(httpStatusCodes.OK, payload); - testAction( + return testAction( actions.startPolling, {}, {}, @@ -35,16 +35,13 @@ describe('error tracking actions', () => { { type: types.SET_LOADING, payload: false }, ], [{ type: 'stopPolling' }], - () => { - done(); - }, ); }); - it('should show flash on API error', (done) => { + it('should show flash on API error', async () => { mock.onGet().reply(httpStatusCodes.BAD_REQUEST); - testAction( + await testAction( actions.startPolling, {}, {}, @@ -53,11 +50,8 @@ describe('error tracking actions', () => { { type: types.SET_LOADING, payload: false }, ], [], - () => { - expect(createFlash).toHaveBeenCalledTimes(1); - done(); - }, ); + expect(createFlash).toHaveBeenCalledTimes(1); }); }); diff --git a/spec/frontend/error_tracking_settings/store/actions_spec.js b/spec/frontend/error_tracking_settings/store/actions_spec.js index 1b9be042dd4..bcd816c2ae0 100644 --- a/spec/frontend/error_tracking_settings/store/actions_spec.js +++ b/spec/frontend/error_tracking_settings/store/actions_spec.js @@ -27,9 +27,9 @@ describe('error tracking settings actions', () => { refreshCurrentPage.mockClear(); }); - it('should request and transform the project list', (done) => { + it('should request and transform the project list', async () => { mock.onGet(TEST_HOST).reply(() => [200, { projects: projectList }]); - testAction( + await testAction( actions.fetchProjects, null, state, @@ -41,16 +41,13 @@ describe('error tracking settings actions', () => { payload: projectList.map(convertObjectPropsToCamelCase), }, ], - () => { - expect(mock.history.get.length).toBe(1); - done(); - }, ); + expect(mock.history.get.length).toBe(1); }); - it('should handle a server error', (done) => { + it('should handle a server error', async () => { mock.onGet(`${TEST_HOST}.json`).reply(() => [400]); - testAction( + await testAction( actions.fetchProjects, null, state, @@ -61,27 +58,23 @@ describe('error tracking settings actions', () => { type: 'receiveProjectsError', }, ], - () => { - expect(mock.history.get.length).toBe(1); - done(); - }, ); + expect(mock.history.get.length).toBe(1); }); - it('should request projects correctly', (done) => { - testAction( + it('should request projects correctly', () => { + return testAction( actions.requestProjects, null, state, [{ type: types.SET_PROJECTS_LOADING, payload: true }, { type: types.RESET_CONNECT }], [], - done, ); }); - it('should receive projects correctly', (done) => { + it('should receive projects correctly', () => { const testPayload = []; - testAction( + return testAction( actions.receiveProjectsSuccess, testPayload, state, @@ -91,13 +84,12 @@ describe('error tracking settings actions', () => { { type: types.SET_PROJECTS_LOADING, payload: false }, ], [], - done, ); }); - it('should handle errors when receiving projects', (done) => { + it('should handle errors when receiving projects', () => { const testPayload = []; - testAction( + return testAction( actions.receiveProjectsError, testPayload, state, @@ -107,7 +99,6 @@ describe('error tracking settings actions', () => { { type: types.SET_PROJECTS_LOADING, payload: false }, ], [], - done, ); }); }); @@ -126,18 +117,16 @@ describe('error tracking settings actions', () => { mock.restore(); }); - it('should save the page', (done) => { + it('should save the page', async () => { mock.onPatch(TEST_HOST).reply(200); - testAction(actions.updateSettings, null, state, [], [{ type: 'requestSettings' }], () => { - expect(mock.history.patch.length).toBe(1); - expect(refreshCurrentPage).toHaveBeenCalled(); - done(); - }); + await testAction(actions.updateSettings, null, state, [], [{ type: 'requestSettings' }]); + expect(mock.history.patch.length).toBe(1); + expect(refreshCurrentPage).toHaveBeenCalled(); }); - it('should handle a server error', (done) => { + it('should handle a server error', async () => { mock.onPatch(TEST_HOST).reply(400); - testAction( + await testAction( actions.updateSettings, null, state, @@ -149,57 +138,50 @@ describe('error tracking settings actions', () => { payload: new Error('Request failed with status code 400'), }, ], - () => { - expect(mock.history.patch.length).toBe(1); - done(); - }, ); + expect(mock.history.patch.length).toBe(1); }); - it('should request to save the page', (done) => { - testAction( + it('should request to save the page', () => { + return testAction( actions.requestSettings, null, state, [{ type: types.UPDATE_SETTINGS_LOADING, payload: true }], [], - done, ); }); - it('should handle errors when requesting to save the page', (done) => { - testAction( + it('should handle errors when requesting to save the page', () => { + return testAction( actions.receiveSettingsError, {}, state, [{ type: types.UPDATE_SETTINGS_LOADING, payload: false }], [], - done, ); }); }); describe('generic actions to update the store', () => { const testData = 'test'; - it('should reset the `connect success` flag when updating the api host', (done) => { - testAction( + it('should reset the `connect success` flag when updating the api host', () => { + return testAction( actions.updateApiHost, testData, state, [{ type: types.UPDATE_API_HOST, payload: testData }, { type: types.RESET_CONNECT }], [], - done, ); }); - it('should reset the `connect success` flag when updating the token', (done) => { - testAction( + it('should reset the `connect success` flag when updating the token', () => { + return testAction( actions.updateToken, testData, state, [{ type: types.UPDATE_TOKEN, payload: testData }, { type: types.RESET_CONNECT }], [], - done, ); }); diff --git a/spec/frontend/feature_flags/store/edit/actions_spec.js b/spec/frontend/feature_flags/store/edit/actions_spec.js index 12fccd79170..b6114cb0c9f 100644 --- a/spec/frontend/feature_flags/store/edit/actions_spec.js +++ b/spec/frontend/feature_flags/store/edit/actions_spec.js @@ -40,7 +40,7 @@ describe('Feature flags Edit Module actions', () => { }); describe('success', () => { - it('dispatches requestUpdateFeatureFlag and receiveUpdateFeatureFlagSuccess ', (done) => { + it('dispatches requestUpdateFeatureFlag and receiveUpdateFeatureFlagSuccess ', () => { const featureFlag = { name: 'name', description: 'description', @@ -57,7 +57,7 @@ describe('Feature flags Edit Module actions', () => { }; mock.onPut(mockedState.endpoint, mapStrategiesToRails(featureFlag)).replyOnce(200); - testAction( + return testAction( updateFeatureFlag, featureFlag, mockedState, @@ -70,16 +70,15 @@ describe('Feature flags Edit Module actions', () => { type: 'receiveUpdateFeatureFlagSuccess', }, ], - done, ); }); }); describe('error', () => { - it('dispatches requestUpdateFeatureFlag and receiveUpdateFeatureFlagError ', (done) => { + it('dispatches requestUpdateFeatureFlag and receiveUpdateFeatureFlagError ', () => { mock.onPut(`${TEST_HOST}/endpoint.json`).replyOnce(500, { message: [] }); - testAction( + return testAction( updateFeatureFlag, { name: 'feature_flag', @@ -97,28 +96,26 @@ describe('Feature flags Edit Module actions', () => { payload: { message: [] }, }, ], - done, ); }); }); }); describe('requestUpdateFeatureFlag', () => { - it('should commit REQUEST_UPDATE_FEATURE_FLAG mutation', (done) => { - testAction( + it('should commit REQUEST_UPDATE_FEATURE_FLAG mutation', () => { + return testAction( requestUpdateFeatureFlag, null, mockedState, [{ type: types.REQUEST_UPDATE_FEATURE_FLAG }], [], - done, ); }); }); describe('receiveUpdateFeatureFlagSuccess', () => { - it('should commit RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS mutation', (done) => { - testAction( + it('should commit RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS mutation', () => { + return testAction( receiveUpdateFeatureFlagSuccess, null, mockedState, @@ -128,20 +125,18 @@ describe('Feature flags Edit Module actions', () => { }, ], [], - done, ); }); }); describe('receiveUpdateFeatureFlagError', () => { - it('should commit RECEIVE_UPDATE_FEATURE_FLAG_ERROR mutation', (done) => { - testAction( + it('should commit RECEIVE_UPDATE_FEATURE_FLAG_ERROR mutation', () => { + return testAction( receiveUpdateFeatureFlagError, 'There was an error', mockedState, [{ type: types.RECEIVE_UPDATE_FEATURE_FLAG_ERROR, payload: 'There was an error' }], [], - done, ); }); }); @@ -159,10 +154,10 @@ describe('Feature flags Edit Module actions', () => { }); describe('success', () => { - it('dispatches requestFeatureFlag and receiveFeatureFlagSuccess ', (done) => { + it('dispatches requestFeatureFlag and receiveFeatureFlagSuccess ', () => { mock.onGet(`${TEST_HOST}/endpoint.json`).replyOnce(200, { id: 1 }); - testAction( + return testAction( fetchFeatureFlag, { id: 1 }, mockedState, @@ -176,16 +171,15 @@ describe('Feature flags Edit Module actions', () => { payload: { id: 1 }, }, ], - done, ); }); }); describe('error', () => { - it('dispatches requestFeatureFlag and receiveUpdateFeatureFlagError ', (done) => { + it('dispatches requestFeatureFlag and receiveUpdateFeatureFlagError ', () => { mock.onGet(`${TEST_HOST}/endpoint.json`, {}).replyOnce(500, {}); - testAction( + return testAction( fetchFeatureFlag, null, mockedState, @@ -198,41 +192,38 @@ describe('Feature flags Edit Module actions', () => { type: 'receiveFeatureFlagError', }, ], - done, ); }); }); }); describe('requestFeatureFlag', () => { - it('should commit REQUEST_FEATURE_FLAG mutation', (done) => { - testAction( + it('should commit REQUEST_FEATURE_FLAG mutation', () => { + return testAction( requestFeatureFlag, null, mockedState, [{ type: types.REQUEST_FEATURE_FLAG }], [], - done, ); }); }); describe('receiveFeatureFlagSuccess', () => { - it('should commit RECEIVE_FEATURE_FLAG_SUCCESS mutation', (done) => { - testAction( + it('should commit RECEIVE_FEATURE_FLAG_SUCCESS mutation', () => { + return testAction( receiveFeatureFlagSuccess, { id: 1 }, mockedState, [{ type: types.RECEIVE_FEATURE_FLAG_SUCCESS, payload: { id: 1 } }], [], - done, ); }); }); describe('receiveFeatureFlagError', () => { - it('should commit RECEIVE_FEATURE_FLAG_ERROR mutation', (done) => { - testAction( + it('should commit RECEIVE_FEATURE_FLAG_ERROR mutation', () => { + return testAction( receiveFeatureFlagError, null, mockedState, @@ -242,20 +233,18 @@ describe('Feature flags Edit Module actions', () => { }, ], [], - done, ); }); }); describe('toggelActive', () => { - it('should commit TOGGLE_ACTIVE mutation', (done) => { - testAction( + it('should commit TOGGLE_ACTIVE mutation', () => { + return testAction( toggleActive, true, mockedState, [{ type: types.TOGGLE_ACTIVE, payload: true }], [], - done, ); }); }); diff --git a/spec/frontend/feature_flags/store/index/actions_spec.js b/spec/frontend/feature_flags/store/index/actions_spec.js index a59f99f538c..ce62c3b0473 100644 --- a/spec/frontend/feature_flags/store/index/actions_spec.js +++ b/spec/frontend/feature_flags/store/index/actions_spec.js @@ -32,14 +32,13 @@ describe('Feature flags actions', () => { }); describe('setFeatureFlagsOptions', () => { - it('should commit SET_FEATURE_FLAGS_OPTIONS mutation', (done) => { - testAction( + it('should commit SET_FEATURE_FLAGS_OPTIONS mutation', () => { + return testAction( setFeatureFlagsOptions, { page: '1', scope: 'all' }, mockedState, [{ type: types.SET_FEATURE_FLAGS_OPTIONS, payload: { page: '1', scope: 'all' } }], [], - done, ); }); }); @@ -57,10 +56,10 @@ describe('Feature flags actions', () => { }); describe('success', () => { - it('dispatches requestFeatureFlags and receiveFeatureFlagsSuccess ', (done) => { + it('dispatches requestFeatureFlags and receiveFeatureFlagsSuccess ', () => { mock.onGet(`${TEST_HOST}/endpoint.json`).replyOnce(200, getRequestData, {}); - testAction( + return testAction( fetchFeatureFlags, null, mockedState, @@ -74,16 +73,15 @@ describe('Feature flags actions', () => { type: 'receiveFeatureFlagsSuccess', }, ], - done, ); }); }); describe('error', () => { - it('dispatches requestFeatureFlags and receiveFeatureFlagsError ', (done) => { + it('dispatches requestFeatureFlags and receiveFeatureFlagsError ', () => { mock.onGet(`${TEST_HOST}/endpoint.json`, {}).replyOnce(500, {}); - testAction( + return testAction( fetchFeatureFlags, null, mockedState, @@ -96,28 +94,26 @@ describe('Feature flags actions', () => { type: 'receiveFeatureFlagsError', }, ], - done, ); }); }); }); describe('requestFeatureFlags', () => { - it('should commit RECEIVE_FEATURE_FLAGS_SUCCESS mutation', (done) => { - testAction( + it('should commit RECEIVE_FEATURE_FLAGS_SUCCESS mutation', () => { + return testAction( requestFeatureFlags, null, mockedState, [{ type: types.REQUEST_FEATURE_FLAGS }], [], - done, ); }); }); describe('receiveFeatureFlagsSuccess', () => { - it('should commit RECEIVE_FEATURE_FLAGS_SUCCESS mutation', (done) => { - testAction( + it('should commit RECEIVE_FEATURE_FLAGS_SUCCESS mutation', () => { + return testAction( receiveFeatureFlagsSuccess, { data: getRequestData, headers: {} }, mockedState, @@ -128,20 +124,18 @@ describe('Feature flags actions', () => { }, ], [], - done, ); }); }); describe('receiveFeatureFlagsError', () => { - it('should commit RECEIVE_FEATURE_FLAGS_ERROR mutation', (done) => { - testAction( + it('should commit RECEIVE_FEATURE_FLAGS_ERROR mutation', () => { + return testAction( receiveFeatureFlagsError, null, mockedState, [{ type: types.RECEIVE_FEATURE_FLAGS_ERROR }], [], - done, ); }); }); @@ -159,10 +153,10 @@ describe('Feature flags actions', () => { }); describe('success', () => { - it('dispatches requestRotateInstanceId and receiveRotateInstanceIdSuccess ', (done) => { + it('dispatches requestRotateInstanceId and receiveRotateInstanceIdSuccess ', () => { mock.onPost(`${TEST_HOST}/endpoint.json`).replyOnce(200, rotateData, {}); - testAction( + return testAction( rotateInstanceId, null, mockedState, @@ -176,16 +170,15 @@ describe('Feature flags actions', () => { type: 'receiveRotateInstanceIdSuccess', }, ], - done, ); }); }); describe('error', () => { - it('dispatches requestRotateInstanceId and receiveRotateInstanceIdError ', (done) => { + it('dispatches requestRotateInstanceId and receiveRotateInstanceIdError ', () => { mock.onGet(`${TEST_HOST}/endpoint.json`, {}).replyOnce(500, {}); - testAction( + return testAction( rotateInstanceId, null, mockedState, @@ -198,28 +191,26 @@ describe('Feature flags actions', () => { type: 'receiveRotateInstanceIdError', }, ], - done, ); }); }); }); describe('requestRotateInstanceId', () => { - it('should commit REQUEST_ROTATE_INSTANCE_ID mutation', (done) => { - testAction( + it('should commit REQUEST_ROTATE_INSTANCE_ID mutation', () => { + return testAction( requestRotateInstanceId, null, mockedState, [{ type: types.REQUEST_ROTATE_INSTANCE_ID }], [], - done, ); }); }); describe('receiveRotateInstanceIdSuccess', () => { - it('should commit RECEIVE_ROTATE_INSTANCE_ID_SUCCESS mutation', (done) => { - testAction( + it('should commit RECEIVE_ROTATE_INSTANCE_ID_SUCCESS mutation', () => { + return testAction( receiveRotateInstanceIdSuccess, { data: rotateData, headers: {} }, mockedState, @@ -230,20 +221,18 @@ describe('Feature flags actions', () => { }, ], [], - done, ); }); }); describe('receiveRotateInstanceIdError', () => { - it('should commit RECEIVE_ROTATE_INSTANCE_ID_ERROR mutation', (done) => { - testAction( + it('should commit RECEIVE_ROTATE_INSTANCE_ID_ERROR mutation', () => { + return testAction( receiveRotateInstanceIdError, null, mockedState, [{ type: types.RECEIVE_ROTATE_INSTANCE_ID_ERROR }], [], - done, ); }); }); @@ -262,10 +251,10 @@ describe('Feature flags actions', () => { mock.restore(); }); describe('success', () => { - it('dispatches updateFeatureFlag and receiveUpdateFeatureFlagSuccess', (done) => { + it('dispatches updateFeatureFlag and receiveUpdateFeatureFlagSuccess', () => { mock.onPut(featureFlag.update_path).replyOnce(200, featureFlag, {}); - testAction( + return testAction( toggleFeatureFlag, featureFlag, mockedState, @@ -280,15 +269,15 @@ describe('Feature flags actions', () => { type: 'receiveUpdateFeatureFlagSuccess', }, ], - done, ); }); }); + describe('error', () => { - it('dispatches updateFeatureFlag and receiveUpdateFeatureFlagSuccess', (done) => { + it('dispatches updateFeatureFlag and receiveUpdateFeatureFlagSuccess', () => { mock.onPut(featureFlag.update_path).replyOnce(500); - testAction( + return testAction( toggleFeatureFlag, featureFlag, mockedState, @@ -303,7 +292,6 @@ describe('Feature flags actions', () => { type: 'receiveUpdateFeatureFlagError', }, ], - done, ); }); }); @@ -315,8 +303,8 @@ describe('Feature flags actions', () => { })); }); - it('commits UPDATE_FEATURE_FLAG with the given flag', (done) => { - testAction( + it('commits UPDATE_FEATURE_FLAG with the given flag', () => { + return testAction( updateFeatureFlag, featureFlag, mockedState, @@ -327,7 +315,6 @@ describe('Feature flags actions', () => { }, ], [], - done, ); }); }); @@ -338,8 +325,8 @@ describe('Feature flags actions', () => { })); }); - it('commits RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS with the given flag', (done) => { - testAction( + it('commits RECEIVE_UPDATE_FEATURE_FLAG_SUCCESS with the given flag', () => { + return testAction( receiveUpdateFeatureFlagSuccess, featureFlag, mockedState, @@ -350,7 +337,6 @@ describe('Feature flags actions', () => { }, ], [], - done, ); }); }); @@ -361,8 +347,8 @@ describe('Feature flags actions', () => { })); }); - it('commits RECEIVE_UPDATE_FEATURE_FLAG_ERROR with the given flag id', (done) => { - testAction( + it('commits RECEIVE_UPDATE_FEATURE_FLAG_ERROR with the given flag id', () => { + return testAction( receiveUpdateFeatureFlagError, featureFlag.id, mockedState, @@ -373,22 +359,20 @@ describe('Feature flags actions', () => { }, ], [], - done, ); }); }); describe('clearAlert', () => { - it('should commit RECEIVE_CLEAR_ALERT', (done) => { + it('should commit RECEIVE_CLEAR_ALERT', () => { const alertIndex = 3; - testAction( + return testAction( clearAlert, alertIndex, mockedState, [{ type: 'RECEIVE_CLEAR_ALERT', payload: alertIndex }], [], - done, ); }); }); diff --git a/spec/frontend/feature_flags/store/new/actions_spec.js b/spec/frontend/feature_flags/store/new/actions_spec.js index 7900b200eb2..1dcd2da1d93 100644 --- a/spec/frontend/feature_flags/store/new/actions_spec.js +++ b/spec/frontend/feature_flags/store/new/actions_spec.js @@ -33,7 +33,7 @@ describe('Feature flags New Module Actions', () => { }); describe('success', () => { - it('dispatches requestCreateFeatureFlag and receiveCreateFeatureFlagSuccess ', (done) => { + it('dispatches requestCreateFeatureFlag and receiveCreateFeatureFlagSuccess ', () => { const actionParams = { name: 'name', description: 'description', @@ -50,7 +50,7 @@ describe('Feature flags New Module Actions', () => { }; mock.onPost(mockedState.endpoint, mapStrategiesToRails(actionParams)).replyOnce(200); - testAction( + return testAction( createFeatureFlag, actionParams, mockedState, @@ -63,13 +63,12 @@ describe('Feature flags New Module Actions', () => { type: 'receiveCreateFeatureFlagSuccess', }, ], - done, ); }); }); describe('error', () => { - it('dispatches requestCreateFeatureFlag and receiveCreateFeatureFlagError ', (done) => { + it('dispatches requestCreateFeatureFlag and receiveCreateFeatureFlagError ', () => { const actionParams = { name: 'name', description: 'description', @@ -88,7 +87,7 @@ describe('Feature flags New Module Actions', () => { .onPost(mockedState.endpoint, mapStrategiesToRails(actionParams)) .replyOnce(500, { message: [] }); - testAction( + return testAction( createFeatureFlag, actionParams, mockedState, @@ -102,28 +101,26 @@ describe('Feature flags New Module Actions', () => { payload: { message: [] }, }, ], - done, ); }); }); }); describe('requestCreateFeatureFlag', () => { - it('should commit REQUEST_CREATE_FEATURE_FLAG mutation', (done) => { - testAction( + it('should commit REQUEST_CREATE_FEATURE_FLAG mutation', () => { + return testAction( requestCreateFeatureFlag, null, mockedState, [{ type: types.REQUEST_CREATE_FEATURE_FLAG }], [], - done, ); }); }); describe('receiveCreateFeatureFlagSuccess', () => { - it('should commit RECEIVE_CREATE_FEATURE_FLAG_SUCCESS mutation', (done) => { - testAction( + it('should commit RECEIVE_CREATE_FEATURE_FLAG_SUCCESS mutation', () => { + return testAction( receiveCreateFeatureFlagSuccess, null, mockedState, @@ -133,20 +130,18 @@ describe('Feature flags New Module Actions', () => { }, ], [], - done, ); }); }); describe('receiveCreateFeatureFlagError', () => { - it('should commit RECEIVE_CREATE_FEATURE_FLAG_ERROR mutation', (done) => { - testAction( + it('should commit RECEIVE_CREATE_FEATURE_FLAG_ERROR mutation', () => { + return testAction( receiveCreateFeatureFlagError, 'There was an error', mockedState, [{ type: types.RECEIVE_CREATE_FEATURE_FLAG_ERROR, payload: 'There was an error' }], [], - done, ); }); }); diff --git a/spec/frontend/pager_spec.js b/spec/frontend/pager_spec.js index 043ea470436..9df69124d66 100644 --- a/spec/frontend/pager_spec.js +++ b/spec/frontend/pager_spec.js @@ -68,34 +68,34 @@ describe('pager', () => { it('shows loader while loading next page', async () => { mockSuccess(); - jest.spyOn(Pager.loading, 'show').mockImplementation(() => {}); + jest.spyOn(Pager.$loading, 'show').mockImplementation(() => {}); Pager.getOld(); await waitForPromises(); - expect(Pager.loading.show).toHaveBeenCalled(); + expect(Pager.$loading.show).toHaveBeenCalled(); }); it('hides loader on success', async () => { mockSuccess(); - jest.spyOn(Pager.loading, 'hide').mockImplementation(() => {}); + jest.spyOn(Pager.$loading, 'hide').mockImplementation(() => {}); Pager.getOld(); await waitForPromises(); - expect(Pager.loading.hide).toHaveBeenCalled(); + expect(Pager.$loading.hide).toHaveBeenCalled(); }); it('hides loader on error', async () => { mockError(); - jest.spyOn(Pager.loading, 'hide').mockImplementation(() => {}); + jest.spyOn(Pager.$loading, 'hide').mockImplementation(() => {}); Pager.getOld(); await waitForPromises(); - expect(Pager.loading.hide).toHaveBeenCalled(); + expect(Pager.$loading.hide).toHaveBeenCalled(); }); it('sends request to url with offset and limit params', async () => { @@ -122,12 +122,12 @@ describe('pager', () => { Pager.limit = 20; mockSuccess(1); - jest.spyOn(Pager.loading, 'hide').mockImplementation(() => {}); + jest.spyOn(Pager.$loading, 'hide').mockImplementation(() => {}); Pager.getOld(); await waitForPromises(); - expect(Pager.loading.hide).toHaveBeenCalled(); + expect(Pager.$loading.hide).toHaveBeenCalled(); expect(Pager.disable).toBe(true); }); @@ -175,5 +175,46 @@ describe('pager', () => { expect(axios.get).toHaveBeenCalledWith(href, expect.any(Object)); }); }); + + describe('when `container` is passed', () => { + const href = '/some_list'; + const container = '#js-pager'; + let endlessScrollCallback; + + beforeEach(() => { + jest.spyOn(axios, 'get'); + jest.spyOn($.fn, 'endlessScroll').mockImplementation(({ callback }) => { + endlessScrollCallback = callback; + }); + }); + + describe('when `container` is visible', () => { + it('makes API request', () => { + setFixtures( + `
`, + ); + + Pager.init({ container }); + + endlessScrollCallback(); + + expect(axios.get).toHaveBeenCalledWith(href, expect.any(Object)); + }); + }); + + describe('when `container` is not visible', () => { + it('does not make API request', () => { + setFixtures( + ``, + ); + + Pager.init({ container }); + + endlessScrollCallback(); + + expect(axios.get).not.toHaveBeenCalled(); + }); + }); + }); }); }); diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index f6745bbdac7..85a2fb1e512 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -185,16 +185,6 @@ RSpec.describe Gitlab::Database do end end - describe '.nulls_last_order' do - it { expect(described_class.nulls_last_order('column', 'ASC')).to eq 'column ASC NULLS LAST'} - it { expect(described_class.nulls_last_order('column', 'DESC')).to eq 'column DESC NULLS LAST'} - end - - describe '.nulls_first_order' do - it { expect(described_class.nulls_first_order('column', 'ASC')).to eq 'column ASC NULLS FIRST'} - it { expect(described_class.nulls_first_order('column', 'DESC')).to eq 'column DESC NULLS FIRST'} - end - describe '.db_config_for_connection' do context 'when the regular connection is used' do it 'returns db_config' do diff --git a/spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb index ab3e409f62c..86e7d4e344c 100644 --- a/spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/keyset/connection_generic_keyset_spec.rb @@ -19,8 +19,8 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'last_repository_check_at', column_expression: Project.arel_table[:last_repository_check_at], - order_expression: Gitlab::Database.nulls_last_order('last_repository_check_at', :asc), - reversed_order_expression: Gitlab::Database.nulls_last_order('last_repository_check_at', :desc), + order_expression: Project.arel_table[:last_repository_check_at].asc.nulls_last, + reversed_order_expression: Project.arel_table[:last_repository_check_at].desc.nulls_last, order_direction: :asc, nullable: :nulls_last, distinct: false) @@ -30,8 +30,8 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'last_repository_check_at', column_expression: Project.arel_table[:last_repository_check_at], - order_expression: Gitlab::Database.nulls_last_order('last_repository_check_at', :desc), - reversed_order_expression: Gitlab::Database.nulls_last_order('last_repository_check_at', :asc), + order_expression: Project.arel_table[:last_repository_check_at].desc.nulls_last, + reversed_order_expression: Project.arel_table[:last_repository_check_at].asc.nulls_last, order_direction: :desc, nullable: :nulls_last, distinct: false) diff --git a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb index ab0eeb03428..fcae89a9025 100644 --- a/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb +++ b/spec/lib/gitlab/graphql/pagination/keyset/connection_spec.rb @@ -264,7 +264,6 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do where(:nodes) do [ - lazy { Issue.order(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')) }, lazy { Issue.order(Issue.arel_table[:relative_position].asc.nulls_last) } ] end @@ -279,7 +278,6 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do where(:nodes) do [ - lazy { Issue.order(::Gitlab::Database.nulls_last_order('relative_position', 'DESC')) }, lazy { Issue.order(Issue.arel_table[:relative_position].desc.nulls_last) } ] end @@ -294,7 +292,6 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do where(:nodes) do [ - lazy { Issue.order(::Gitlab::Database.nulls_first_order('relative_position', 'ASC')).order(id: :asc) }, lazy { Issue.order(Issue.arel_table[:relative_position].asc.nulls_first).order(id: :asc) } ] end @@ -309,7 +306,6 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::Connection do where(:nodes) do [ - lazy { Issue.order(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')).order(id: :asc) }, lazy { Issue.order(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :asc) } ] end diff --git a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb index ba1cccf87ce..03f522ae490 100644 --- a/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb +++ b/spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb @@ -115,7 +115,7 @@ RSpec.describe Gitlab::ImportExport::Json::StreamingSerializer do end it 'orders exported issues by custom column(relative_position)' do - expected_issues = exportable.issues.reorder(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')).order(id: :desc).map(&:to_json) + expected_issues = exportable.issues.reorder(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :desc).map(&:to_json) expect(json_writer).to receive(:write_relation_array).with(exportable_path, :issues, expected_issues) diff --git a/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb b/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb index 69384e0c501..778244677ef 100644 --- a/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/column_order_definition_spec.rb @@ -140,8 +140,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do described_class.new( attribute_name: :name, column_expression: Project.arel_table[:name], - order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc), - reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc), + order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last, + reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first, order_direction: :desc, nullable: :nulls_last, # null values are always last distinct: false @@ -161,8 +161,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do described_class.new( attribute_name: :name, column_expression: Project.arel_table[:name], - order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc), - reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc), + order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last, + reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first, order_direction: :desc, nullable: true, distinct: false @@ -175,8 +175,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do described_class.new( attribute_name: :name, column_expression: Project.arel_table[:name], - order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc), - reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc), + order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last, + reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first, order_direction: :desc, nullable: :nulls_last, distinct: true @@ -191,8 +191,8 @@ RSpec.describe Gitlab::Pagination::Keyset::ColumnOrderDefinition do described_class.new( attribute_name: :name, column_expression: Project.arel_table[:name], - order_expression: Gitlab::Database.nulls_last_order('merge_request_metrics.merged_at', :desc), - reversed_order_expression: Gitlab::Database.nulls_first_order('merge_request_metrics.merged_at', :asc), + order_expression: MergeRequest::Metrics.arel_table[:merged_at].desc.nulls_last, + reversed_order_expression: MergeRequest::Metrics.arel_table[:merged_at].asc.nulls_first, order_direction: :desc, nullable: :nulls_last, # null values are always last distinct: false diff --git a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb index 1ddc281f988..832e269a78c 100644 --- a/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/in_operator_optimization/query_builder_spec.rb @@ -121,8 +121,8 @@ RSpec.describe Gitlab::Pagination::Keyset::InOperatorOptimization::QueryBuilder Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: :relative_position, column_expression: Issue.arel_table[:relative_position], - order_expression: Gitlab::Database.nulls_last_order('relative_position', :desc), - reversed_order_expression: Gitlab::Database.nulls_first_order('relative_position', :asc), + order_expression: Issue.arel_table[:relative_position].desc.nulls_last, + reversed_order_expression: Issue.arel_table[:relative_position].asc.nulls_first, order_direction: :desc, nullable: :nulls_last, distinct: false diff --git a/spec/lib/gitlab/pagination/keyset/iterator_spec.rb b/spec/lib/gitlab/pagination/keyset/iterator_spec.rb index 09cbca2c1cb..d62d20d2d2c 100644 --- a/spec/lib/gitlab/pagination/keyset/iterator_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/iterator_spec.rb @@ -19,8 +19,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: column, column_expression: klass.arel_table[column], - order_expression: ::Gitlab::Database.nulls_order(column, direction, nulls_position), - reversed_order_expression: ::Gitlab::Database.nulls_order(column, reverse_direction, reverse_nulls_position), + order_expression: klass.arel_table[column].public_send(direction).public_send(nulls_position), # rubocop:disable GitlabSecurity/PublicSend + reversed_order_expression: klass.arel_table[column].public_send(reverse_direction).public_send(reverse_nulls_position), # rubocop:disable GitlabSecurity/PublicSend order_direction: direction, nullable: nulls_position, distinct: false @@ -99,7 +99,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } - expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')).order(id: :asc).pluck(:relative_position, :id)) + expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].asc.nulls_last).order(id: :asc).pluck(:relative_position, :id)) end end @@ -111,7 +111,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } - expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')).order(id: :desc).pluck(:relative_position, :id)) + expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :desc).pluck(:relative_position, :id)) end end @@ -123,7 +123,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } - expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_first_order('relative_position', 'ASC')).order(id: :asc).pluck(:relative_position, :id)) + expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].asc.nulls_first).order(id: :asc).pluck(:relative_position, :id)) end end @@ -136,7 +136,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } - expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_last_order('relative_position', 'DESC')).order(id: :desc).pluck(:relative_position, :id)) + expect(positions).to eq(project.issues.reorder(Issue.arel_table[:relative_position].desc.nulls_last).order(id: :desc).pluck(:relative_position, :id)) end end diff --git a/spec/lib/gitlab/pagination/keyset/order_spec.rb b/spec/lib/gitlab/pagination/keyset/order_spec.rb index 638ae8089d8..abbb3a21cd4 100644 --- a/spec/lib/gitlab/pagination/keyset/order_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/order_spec.rb @@ -262,8 +262,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'year', column_expression: table['year'], - order_expression: Gitlab::Database.nulls_last_order('year', :asc), - reversed_order_expression: Gitlab::Database.nulls_first_order('year', :desc), + order_expression: table[:year].asc.nulls_last, + reversed_order_expression: table[:year].desc.nulls_first, order_direction: :asc, nullable: :nulls_last, distinct: false @@ -271,8 +271,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'month', column_expression: table['month'], - order_expression: Gitlab::Database.nulls_last_order('month', :asc), - reversed_order_expression: Gitlab::Database.nulls_first_order('month', :desc), + order_expression: table[:month].asc.nulls_last, + reversed_order_expression: table[:month].desc.nulls_first, order_direction: :asc, nullable: :nulls_last, distinct: false @@ -328,8 +328,8 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'year', column_expression: table['year'], - order_expression: Gitlab::Database.nulls_first_order('year', :asc), - reversed_order_expression: Gitlab::Database.nulls_last_order('year', :desc), + order_expression: table[:year].asc.nulls_first, + reversed_order_expression: table[:year].desc.nulls_last, order_direction: :asc, nullable: :nulls_first, distinct: false @@ -337,9 +337,9 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'month', column_expression: table['month'], - order_expression: Gitlab::Database.nulls_first_order('month', :asc), + order_expression: table[:month].asc.nulls_first, order_direction: :asc, - reversed_order_expression: Gitlab::Database.nulls_last_order('month', :desc), + reversed_order_expression: table[:month].desc.nulls_last, nullable: :nulls_first, distinct: false ), diff --git a/spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb b/spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb index 47376a9c387..4f1d380ab0a 100644 --- a/spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/simple_order_builder_spec.rb @@ -94,26 +94,28 @@ RSpec.describe Gitlab::Pagination::Keyset::SimpleOrderBuilder do context "NULLS order given as as an Arel literal" do context 'when NULLS LAST order is given without a tie-breaker' do - let(:scope) { Project.order(::Gitlab::Database.nulls_last_order('created_at', 'ASC')) } + let(:scope) { Project.order(Project.arel_table[:created_at].asc.nulls_last) } it 'sets the column definition for created_at appropriately' do expect(column_definition.attribute_name).to eq('created_at') end it 'orders by primary key' do - expect(sql_with_order).to end_with('ORDER BY "projects".created_at ASC NULLS LAST, "projects"."id" DESC') + expect(sql_with_order) + .to end_with('ORDER BY "projects"."created_at" ASC NULLS LAST, "projects"."id" DESC') end end context 'when NULLS FIRST order is given with a tie-breaker' do - let(:scope) { Issue.order(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')).order(id: :asc) } + let(:scope) { Issue.order(Issue.arel_table[:relative_position].desc.nulls_first).order(id: :asc) } it 'sets the column definition for created_at appropriately' do expect(column_definition.attribute_name).to eq('relative_position') end it 'orders by the given primary key' do - expect(sql_with_order).to end_with('ORDER BY "issues".relative_position DESC NULLS FIRST, "issues"."id" ASC') + expect(sql_with_order) + .to end_with('ORDER BY "issues"."relative_position" DESC NULLS FIRST, "issues"."id" ASC') end end end @@ -159,8 +161,8 @@ RSpec.describe Gitlab::Pagination::Keyset::SimpleOrderBuilder do where(:scope) do [ lazy { Project.order(Arel.sql('projects.updated_at created_at Asc Nulls Last')) }, - lazy { Project.order(::Gitlab::Database.nulls_first_order('created_at', 'ZZZ')) }, - lazy { Project.order(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')) } + lazy { Project.order(Arel.sql('projects.created_at ZZZ NULLS FIRST')) }, + lazy { Project.order(Arel.sql('projects.relative_position ASC NULLS LAST')) } ] end diff --git a/spec/migrations/20210430134202_copy_adoption_snapshot_namespace_spec.rb b/spec/migrations/20210430134202_copy_adoption_snapshot_namespace_spec.rb index 598da495195..ed18820ec8d 100644 --- a/spec/migrations/20210430134202_copy_adoption_snapshot_namespace_spec.rb +++ b/spec/migrations/20210430134202_copy_adoption_snapshot_namespace_spec.rb @@ -36,7 +36,6 @@ RSpec.describe CopyAdoptionSnapshotNamespace, :migration, schema: 20210430124630 runner_configured: true, pipeline_succeeded: true, deploy_succeeded: true, - security_scan_succeeded: true, end_time: Time.zone.now.end_of_month } diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 52ed52de193..6c86db1197f 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -1021,13 +1021,13 @@ RSpec.describe Packages::Package, type: :model do context 'ascending direction' do let(:direction) { :asc } - it { is_expected.to eq('projects.name asc NULLS LAST, "packages_packages"."id" ASC') } + it { is_expected.to eq('"projects"."name" ASC NULLS LAST, "packages_packages"."id" ASC') } end context 'descending direction' do let(:direction) { :desc } - it { is_expected.to eq('projects.name desc NULLS FIRST, "packages_packages"."id" DESC') } + it { is_expected.to eq('"projects"."name" DESC NULLS FIRST, "packages_packages"."id" DESC') } end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 43088739f34..0bb584845c2 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -8235,6 +8235,12 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#inactive?' do + let_it_be_with_reload(:project) { create(:project, name: 'test-project') } + + it_behaves_like 'returns true if project is inactive' + end + private def finish_job(export_job) diff --git a/spec/support/shared_examples/models/project_shared_examples.rb b/spec/support/shared_examples/models/project_shared_examples.rb new file mode 100644 index 00000000000..475ac1da04b --- /dev/null +++ b/spec/support/shared_examples/models/project_shared_examples.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'returns true if project is inactive' do + using RSpec::Parameterized::TableSyntax + + where(:storage_size, :last_activity_at, :expected_result) do + 1.megabyte | 1.month.ago | false + 1.megabyte | 3.years.ago | false + 8.megabytes | 1.month.ago | false + 8.megabytes | 3.years.ago | true + end + + with_them do + before do + stub_application_setting(inactive_projects_min_size_mb: 5) + stub_application_setting(inactive_projects_send_warning_email_after_months: 24) + + project.statistics.storage_size = storage_size + project.last_activity_at = last_activity_at + project.save! + end + + it 'returns expected result' do + expect(project.inactive?).to eq(expected_result) + end + end +end diff --git a/spec/workers/packages/cleanup_package_file_worker_spec.rb b/spec/workers/packages/cleanup_package_file_worker_spec.rb index 702026ed1c7..380e8916d13 100644 --- a/spec/workers/packages/cleanup_package_file_worker_spec.rb +++ b/spec/workers/packages/cleanup_package_file_worker_spec.rb @@ -48,7 +48,7 @@ RSpec.describe Packages::CleanupPackageFileWorker do context 'with an error during the destroy' do before do - expect(worker).to receive(:log_metadata).and_raise('Error!') + allow(worker).to receive(:log_metadata).and_raise('Error!') end it 'handles the error' do @@ -76,7 +76,7 @@ RSpec.describe Packages::CleanupPackageFileWorker do end end - context 'removing the last package file' do + describe 'removing the last package file' do let_it_be(:package_file) { create(:package_file, :pending_destruction, package: package) } it 'deletes the package file and the package' do @@ -101,12 +101,12 @@ RSpec.describe Packages::CleanupPackageFileWorker do end describe '#remaining_work_count' do - before(:context) do - create_list(:package_file, 3, :pending_destruction, package: package) + before_all do + create_list(:package_file, 2, :pending_destruction, package: package) end subject { worker.remaining_work_count } - it { is_expected.to eq(3) } + it { is_expected.to eq(2) } end end