Add latest changes from gitlab-org/gitlab@master

.rubocop.yml

@@ -522,3 +522,9 @@ Migration/ComplexIndexesRequireName:
   Exclude:
     - !ruby/regexp /\Adb\/(post_)?migrate\/201.*\.rb\z/
     - !ruby/regexp /\Adb\/(post_)?migrate\/20200[1-7].*\.rb\z/
+
+Migration/ReferToIndexByName:
+  Exclude:
+    - !ruby/regexp /\Adb\/(post_)?migrate\/201.*\.rb\z/
+    - !ruby/regexp /\Adb\/(post_)?migrate\/20200[1-7].*\.rb\z/
+    - !ruby/regexp /\Aee\/db\/geo\/(post_)?migrate\/201.*\.rb\z/

app/assets/javascripts/diffs/components/commit_item.vue

@@ -149,14 +149,13 @@ export default {
 
         <span class="commit-row-message d-block d-sm-none">&middot; {{ commit.short_id }}</span>
 
-        <button
+        <gl-button
           v-if="commit.description_html && collapsible"
-          class="text-expander js-toggle-button"
+          class="js-toggle-button"
-          type="button"
+          size="small"
+          icon="ellipsis_h"
           :aria-label="__('Toggle commit description')"
-        >
+        />
-          <gl-icon :size="12" name="ellipsis_h" />
-        </button>
 
         <div class="committer">
           <a

app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merged.vue

@@ -1,6 +1,6 @@
 <script>
 /* eslint-disable @gitlab/vue-require-i18n-strings */
-import { GlLoadingIcon } from '@gitlab/ui';
+import { GlLoadingIcon, GlButton } from '@gitlab/ui';
 import { deprecatedCreateFlash as Flash } from '~/flash';
 import tooltip from '~/vue_shared/directives/tooltip';
 import { s__, __ } from '~/locale';
@@ -19,6 +19,7 @@ export default {
     statusIcon,
     ClipboardButton,
     GlLoadingIcon,
+    GlButton,
   },
   props: {
     mr: {
@@ -112,48 +113,52 @@ export default {
           :date-title="mr.metrics.mergedAt"
           :date-readable="mr.metrics.readableMergedAt"
         />
-        <a
+        <gl-button
           v-if="mr.canRevertInCurrentMR"
           v-tooltip
           :title="revertTitle"
-          class="btn btn-close btn-sm"
+          size="small"
+          category="secondary"
+          variant="warning"
           href="#modal-revert-commit"
           data-toggle="modal"
           data-container="body"
         >
           {{ revertLabel }}
-        </a>
+        </gl-button>
-        <a
+        <gl-button
           v-else-if="mr.revertInForkPath"
           v-tooltip
           :href="mr.revertInForkPath"
           :title="revertTitle"
-          class="btn btn-close btn-sm"
+          size="small"
+          category="secondary"
+          variant="warning"
           data-method="post"
         >
           {{ revertLabel }}
-        </a>
+        </gl-button>
-        <a
+        <gl-button
           v-if="mr.canCherryPickInCurrentMR"
           v-tooltip
           :title="cherryPickTitle"
-          class="btn btn-default btn-sm"
+          size="small"
           href="#modal-cherry-pick-commit"
           data-toggle="modal"
           data-container="body"
         >
           {{ cherryPickLabel }}
-        </a>
+        </gl-button>
-        <a
+        <gl-button
           v-else-if="mr.cherryPickInForkPath"
           v-tooltip
           :href="mr.cherryPickInForkPath"
           :title="cherryPickTitle"
-          class="btn btn-default btn-sm"
+          size="small"
           data-method="post"
         >
           {{ cherryPickLabel }}
-        </a>
+        </gl-button>
       </div>
       <section class="mr-info-list" data-qa-selector="merged_status_content">
         <p>
@@ -181,14 +186,14 @@ export default {
         </p>
         <p v-if="shouldShowRemoveSourceBranch" class="space-children">
           <span>{{ s__('mrWidget|You can delete the source branch now') }}</span>
-          <button
+          <gl-button
             :disabled="isMakingRequest"
-            type="button"
+            size="small"
-            class="btn btn-sm btn-default js-remove-branch-button"
+            class="js-remove-branch-button"
             @click="removeSourceBranch"
           >
             {{ s__('mrWidget|Delete source branch') }}
-          </button>
+          </gl-button>
         </p>
         <p v-if="shouldShowSourceBranchRemoving">
           <gl-loading-icon :inline="true" />

app/controllers/sessions_controller.rb

@@ -156,13 +156,13 @@ class SessionsController < Devise::SessionsController
     (options = request.env["warden.options"]) && options[:action] == "unauthenticated"
   end
 
-  # storing sessions per IP lets us check if there are associated multiple
+  # counting sessions per IP lets us check if there are associated multiple
   # anonymous sessions with one IP and prevent situations when there are
   # multiple attempts of logging in
   def store_unauthenticated_sessions
     return if current_user
 
-    Gitlab::AnonymousSession.new(request.remote_ip, session_id: request.session.id).store_session_id_per_ip
+    Gitlab::AnonymousSession.new(request.remote_ip).count_session_ip
   end
 
   # Handle an "initial setup" state, where there's only one user, it's an admin,
@@ -280,7 +280,7 @@ class SessionsController < Devise::SessionsController
   end
 
   def exceeded_anonymous_sessions?
-    Gitlab::AnonymousSession.new(request.remote_ip).stored_sessions >= MAX_FAILED_LOGIN_ATTEMPTS
+    Gitlab::AnonymousSession.new(request.remote_ip).session_count >= MAX_FAILED_LOGIN_ATTEMPTS
   end
 
   def authentication_method

app/graphql/types/release_type.rb

@@ -5,6 +5,8 @@ module Types
     graphql_name 'Release'
     description 'Represents a release'
 
+    connection_type_class(Types::CountableConnectionType)
+
     authorize :read_release
 
     alias_method :release, :object

app/models/note.rb

@@ -563,6 +563,10 @@ class Note < ApplicationRecord
     noteable.author if for_personal_snippet?
   end
 
+  def skip_notification?
+    review.present?
+  end
+
   private
 
   # Using this method followed by a call to `save` may result in ActiveRecord::RecordNotUnique exception

app/models/project_services/jira_service.rb

@@ -5,6 +5,7 @@ class JiraService < IssueTrackerService
   include Gitlab::Routing
   include ApplicationHelper
   include ActionView::Helpers::AssetUrlHelper
+  include Gitlab::Utils::StrongMemoize
 
   PROJECTS_PER_PAGE = 50
 
@@ -32,6 +33,7 @@ class JiraService < IssueTrackerService
   data_field :username, :password, :url, :api_url, :jira_issue_transition_id, :project_key, :issues_enabled
 
   before_update :reset_password
+  after_commit :update_deployment_type, on: [:create, :update], if: :update_deployment_type?
 
   enum comment_detail: {
     standard: 1,
@@ -212,7 +214,7 @@ class JiraService < IssueTrackerService
   end
 
   def test(_)
-    result = test_settings
+    result = server_info
     success = result.present?
     result = @error&.message unless success
 
@@ -231,10 +233,10 @@ class JiraService < IssueTrackerService
 
   private
 
-  def test_settings
+  def server_info
-    return unless client_url.present?
+    strong_memoize(:server_info) do
-
+      client_url.present? ? jira_request { client.ServerInfo.all.attrs } : nil
-    jira_request { client.ServerInfo.all.attrs }
+    end
   end
 
   def can_cross_reference?(noteable)
@@ -436,6 +438,25 @@ class JiraService < IssueTrackerService
     url_changed?
   end
 
+  def update_deployment_type?
+    api_url_changed? || url_changed? || username_changed? || password_changed?
+  end
+
+  def update_deployment_type
+    clear_memoization(:server_info) # ensure we run the request when we try to update deployment type
+    results = server_info
+    return data_fields.deployment_unknown! unless results.present?
+
+    case results['deploymentType']
+    when 'Server'
+      data_fields.deployment_server!
+    when 'Cloud'
+      data_fields.deployment_cloud!
+    else
+      data_fields.deployment_unknown!
+    end
+  end
+
   def self.event_description(event)
     case event
     when "merge_request", "merge_request_events"

app/uploaders/object_storage.rb

@@ -178,10 +178,14 @@ module ObjectStorage
       end
 
       def workhorse_authorize(has_length:, maximum_size: nil)
-        if self.object_store_enabled? && self.direct_upload_enabled?
+        {}.tap do |hash|
-          { RemoteObject: workhorse_remote_upload_options(has_length: has_length, maximum_size: maximum_size) }
+          if self.object_store_enabled? && self.direct_upload_enabled?
-        else
+            hash[:RemoteObject] = workhorse_remote_upload_options(has_length: has_length, maximum_size: maximum_size)
-          { TempPath: workhorse_local_upload_path }
+          else
+            hash[:TempPath] = workhorse_local_upload_path
+          end
+
+          hash[:MaximumSize] = maximum_size if maximum_size.present?
         end
       end
 

app/workers/new_note_worker.rb

@@ -13,17 +13,11 @@ class NewNoteWorker # rubocop:disable Scalability/IdempotentWorker
   # rubocop: disable CodeReuse/ActiveRecord
   def perform(note_id, _params = {})
     if note = Note.find_by(id: note_id)
-      NotificationService.new.new_note(note) unless skip_notification?(note)
+      NotificationService.new.new_note(note) unless note.skip_notification?
       Notes::PostProcessService.new(note).execute
     else
       Gitlab::AppLogger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job")
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord
-
-  private
-
-  def skip_notification?(note)
-    note.review.present?
-  end
 end

changelogs/unreleased/214241-update-releases-page-to-use-graphql-endpoint.yml

@@ -0,0 +1,5 @@
+---
+title: Add total count to GraphQL release data
+merge_request: 40147
+author:
+type: added

changelogs/unreleased/229223-save-jira-server-type-on-test.yml

@@ -0,0 +1,5 @@
+---
+title: Store deployment_type of Jira server in jira_tracker_data table
+merge_request: 37003
+author:
+type: changed

changelogs/unreleased/229336-diffs-collapsible-button.yml

@@ -0,0 +1,5 @@
+---
+title: Update commit toggle description button to gl-button
+merge_request: 40524
+author:
+type: changed

changelogs/unreleased/238964-fj-track-global-editing-actions.yml

@@ -0,0 +1,5 @@
+---
+title: Track edit by editor action for Usage Ping
+merge_request: 40232
+author:
+type: changed

changelogs/unreleased/eb-return-filesize-authorize.yml

@@ -0,0 +1,5 @@
+---
+title: Include max artifact size in authorize response
+merge_request: 37632
+author:
+type: added

changelogs/unreleased/reduce-redis-storage-session-lookup.yml

@@ -0,0 +1,5 @@
+---
+title: Reduce storage requirements for keeping track of pre-logged-in sessions
+merge_request: 40336
+author:
+type: performance

config/initializers/warden.rb

@@ -19,7 +19,7 @@ Rails.application.configure do |config|
 
   Warden::Manager.after_authentication(scope: :user) do |user, auth, opts|
     ActiveSession.cleanup(user)
-    Gitlab::AnonymousSession.new(auth.request.remote_ip, session_id: auth.request.session.id).cleanup_session_per_ip_entries
+    Gitlab::AnonymousSession.new(auth.request.remote_ip).cleanup_session_per_ip_count
   end
 
   Warden::Manager.after_set_user(scope: :user, only: :fetch) do |user, auth, opts|

db/migrate/20200805071842_add_index_on_end_date_and_namespace_id_to_gitlab_subscriptions.rb

@@ -12,6 +12,7 @@ class AddIndexOnEndDateAndNamespaceIdToGitlabSubscriptions < ActiveRecord::Migra
   end
 
   def down
-    remove_concurrent_index :gitlab_subscriptions, [:end_date, :namespace_id]
+    remove_concurrent_index :gitlab_subscriptions, [:end_date, :namespace_id],
+      name: 'index_gitlab_subscriptions_on_end_date_and_namespace_id'
   end
 end

db/migrate/20200820204041_create_ci_platform_metrics.rb

@@ -22,6 +22,6 @@ class CreateCiPlatformMetrics < ActiveRecord::Migration[6.0]
 
   def down
     drop_table :ci_platform_metrics
-    remove_concurrent_index :ci_variables, :key
+    remove_concurrent_index :ci_variables, :key, name: 'index_ci_variables_on_key'
   end
 end

doc/api/graphql/reference/gitlab_schema.graphql

@@ -13282,6 +13282,11 @@ type ReleaseAssets {
 The connection type for Release.
 """
 type ReleaseConnection {
+  """
+  Total count of collection
+  """
+  count: Int!
+
   """
   A list of edges.
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -38872,6 +38872,24 @@
           "name": "ReleaseConnection",
           "description": "The connection type for Release.",
           "fields": [
+            {
+              "name": "count",
+              "description": "Total count of collection",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "Int",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "edges",
               "description": "A list of edges.",

doc/user/analytics/value_stream_analytics.md

@@ -350,15 +350,6 @@ administrator can open a Rails console and disable it with the following command
 Feature.disable(:cycle_analytics_scatterplot_enabled)
 ```
 
-### Disabling chart median line
-
-This chart's median line is enabled by default. If you have a self-managed instance, an
-administrator can open a Rails console and disable it with the following command:
-
-```ruby
-Feature.disable(:cycle_analytics_scatterplot_median_enabled)
-```
-
 ## Type of work - Tasks by type chart
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/32421) in GitLab 12.10.

doc/user/packages/npm_registry/index.md

@@ -308,7 +308,7 @@ stages:
 deploy:
   stage: deploy
   script:
-    - echo '//gitlab.com/api/v4/projects/${CI_PROJECT_ID}/packages/npm/:_authToken=${CI_JOB_TOKEN}'>.npmrc
+    - echo "//gitlab.com/api/v4/projects/${CI_PROJECT_ID}/packages/npm/:_authToken=${CI_JOB_TOKEN}">.npmrc
     - npm publish
 ```
 

lib/gitlab/anonymous_session.rb

@@ -2,35 +2,34 @@
 
 module Gitlab
   class AnonymousSession
-    def initialize(remote_ip, session_id: nil)
+    def initialize(remote_ip)
       @remote_ip = remote_ip
-      @session_id = session_id
     end
 
-    def store_session_id_per_ip
+    def count_session_ip
       Gitlab::Redis::SharedState.with do |redis|
         redis.pipelined do
-          redis.sadd(session_lookup_name, session_id)
+          redis.incr(session_lookup_name)
           redis.expire(session_lookup_name, 24.hours)
         end
       end
     end
 
-    def stored_sessions
+    def session_count
       Gitlab::Redis::SharedState.with do |redis|
-        redis.scard(session_lookup_name)
+        redis.get(session_lookup_name).to_i
       end
     end
 
-    def cleanup_session_per_ip_entries
+    def cleanup_session_per_ip_count
       Gitlab::Redis::SharedState.with do |redis|
-        redis.srem(session_lookup_name, session_id)
+        redis.del(session_lookup_name)
       end
     end
 
     private
 
-    attr_reader :remote_ip, :session_id
+    attr_reader :remote_ip
 
     def session_lookup_name
       @session_lookup_name ||= "#{Gitlab::Redis::SharedState::IP_SESSIONS_LOOKUP_NAMESPACE}:#{remote_ip}"

lib/gitlab/redis/shared_state.rb

@@ -9,7 +9,7 @@ module Gitlab
       SESSION_NAMESPACE = 'session:gitlab'
       USER_SESSIONS_NAMESPACE = 'session:user:gitlab'
       USER_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:user:gitlab'
-      IP_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:ip:gitlab'
+      IP_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:ip:gitlab2'
       DEFAULT_REDIS_SHARED_STATE_URL = 'redis://localhost:6382'
       REDIS_SHARED_STATE_CONFIG_ENV_VAR_NAME = 'GITLAB_REDIS_SHARED_STATE_CONFIG_FILE'
 

lib/gitlab/usage_data_counters/editor_unique_counter.rb

@@ -3,9 +3,10 @@
 module Gitlab
   module UsageDataCounters
     module EditorUniqueCounter
-      EDIT_BY_SNIPPET_EDITOR = :edit_by_snippet_editor
+      EDIT_BY_SNIPPET_EDITOR = 'g_edit_by_snippet_ide'
-      EDIT_BY_SFE = :edit_by_sfe
+      EDIT_BY_SFE = 'g_edit_by_sfe'
-      EDIT_BY_WEB_IDE = :edit_by_web_ide
+      EDIT_BY_WEB_IDE = 'g_edit_by_web_ide'
+      EDIT_CATEGORY = 'ide_edit'
 
       class << self
         def track_web_ide_edit_action(author:, time: Time.zone.now)
@@ -32,16 +33,22 @@ module Gitlab
           count_unique(EDIT_BY_SNIPPET_EDITOR, date_from, date_to)
         end
 
+        def count_edit_using_editor(date_from:, date_to:)
+          events = Gitlab::UsageDataCounters::HLLRedisCounter.events_for_category(EDIT_CATEGORY)
+          count_unique(events, date_from, date_to)
+        end
+
         private
 
         def track_unique_action(action, author, time)
           return unless Feature.enabled?(:track_editor_edit_actions)
+          return unless author
 
-          Gitlab::UsageDataCounters::TrackUniqueActions.track_action(action: action, author_id: author.id, time: time)
+          Gitlab::UsageDataCounters::HLLRedisCounter.track_event(author.id, action, time)
         end
 
-        def count_unique(action, date_from, date_to)
+        def count_unique(actions, date_from, date_to)
-          Gitlab::UsageDataCounters::TrackUniqueActions.count_unique(action: action, date_from: date_from, date_to: date_to)
+          Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(event_names: actions, start_date: date_from, end_date: date_to)
         end
       end
     end

lib/gitlab/usage_data_counters/known_events.yml

@@ -69,3 +69,18 @@
   category: analytics
   redis_slot: analytics
   aggregation: weekly
+- name: g_edit_by_web_ide
+  category: ide_edit
+  redis_slot: edit
+  expiry: 29
+  aggregation: daily
+- name: g_edit_by_sfe
+  category: ide_edit
+  redis_slot: edit
+  expiry: 29
+  aggregation: daily
+- name: g_edit_by_snippet_ide
+  category: ide_edit
+  redis_slot: edit
+  expiry: 29
+  aggregation: daily

locale/gitlab.pot

@@ -24992,9 +24992,6 @@ msgstr ""
 msgid "There was an error while fetching value stream analytics duration data."
 msgstr ""
 
-msgid "There was an error while fetching value stream analytics duration median data."
-msgstr ""
-
 msgid "There was an error with the reCAPTCHA. Please solve the reCAPTCHA again."
 msgstr ""
 

rubocop/cop/migration/refer_to_index_by_name.rb

@@ -0,0 +1,67 @@
+# frozen_string_literal: true
+
+require_relative '../../migration_helpers'
+
+module RuboCop
+  module Cop
+    module Migration
+      class ReferToIndexByName < RuboCop::Cop::Cop
+        include MigrationHelpers
+
+        MSG = 'migration methods that refer to existing indexes must do so by name'
+
+        def_node_matcher :match_index_exists, <<~PATTERN
+          (send _ :index_exists? _ _ (hash $...) ?)
+        PATTERN
+
+        def_node_matcher :match_remove_index, <<~PATTERN
+          (send _ :remove_index _ $_)
+        PATTERN
+
+        def_node_matcher :match_remove_concurrent_index, <<~PATTERN
+          (send _ :remove_concurrent_index _ _ (hash $...) ?)
+        PATTERN
+
+        def_node_matcher :name_option?, <<~PATTERN
+          (pair {(sym :name) (str "name")} _)
+        PATTERN
+
+        def on_def(node)
+          return unless in_migration?(node)
+
+          node.each_descendant(:send) do |send_node|
+            next unless index_exists_offense?(send_node) || removing_index_offense?(send_node)
+
+            add_offense(send_node, location: :selector)
+          end
+        end
+
+        private
+
+        def index_exists_offense?(send_node)
+          match_index_exists(send_node) { |option_nodes| needs_name_option?(option_nodes) }
+        end
+
+        def removing_index_offense?(send_node)
+          remove_index_offense?(send_node) || remove_concurrent_index_offense?(send_node)
+        end
+
+        def remove_index_offense?(send_node)
+          match_remove_index(send_node) do |column_or_options_node|
+            break true unless column_or_options_node.type == :hash
+
+            column_or_options_node.children.none? { |pair| name_option?(pair) }
+          end
+        end
+
+        def remove_concurrent_index_offense?(send_node)
+          match_remove_concurrent_index(send_node) { |option_nodes| needs_name_option?(option_nodes) }
+        end
+
+        def needs_name_option?(option_nodes)
+          option_nodes.empty? || option_nodes.first.none? { |node| name_option?(node) }
+        end
+      end
+    end
+  end
+end

spec/controllers/admin/integrations_controller_spec.rb

@@ -23,9 +23,12 @@ RSpec.describe Admin::IntegrationsController do
   end
 
   describe '#update' do
+    include JiraServiceHelper
+
     let(:integration) { create(:jira_service, :instance) }
 
     before do
+      stub_jira_service_test
       allow(PropagateIntegrationWorker).to receive(:perform_async)
 
       put :update, params: { id: integration.class.to_param, service: { url: url } }

spec/controllers/groups/settings/integrations_controller_spec.rb

@@ -81,10 +81,13 @@ RSpec.describe Groups::Settings::IntegrationsController do
   end
 
   describe '#update' do
+    include JiraServiceHelper
+
     let(:integration) { create(:jira_service, project: nil, group_id: group.id) }
 
     before do
       group.add_owner(user)
+      stub_jira_service_test
 
       put :update, params: { group_id: group, id: integration.class.to_param, service: { url: url } }
     end

spec/controllers/projects/services_controller_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Projects::ServicesController do
+  include JiraServiceHelper
+
   let(:project) { create(:project, :repository) }
   let(:user)    { create(:user) }
   let(:service) { create(:jira_service, project: project) }
@@ -54,8 +56,7 @@ RSpec.describe Projects::ServicesController do
         end
 
         it 'returns success' do
-          stub_request(:get, 'http://example.com/rest/api/2/serverInfo')
+          stub_jira_service_test
-            .to_return(status: 200, body: '{}')
 
           expect(Gitlab::HTTP).to receive(:get).with('/rest/api/2/serverInfo', any_args).and_call_original
 
@@ -66,8 +67,7 @@ RSpec.describe Projects::ServicesController do
       end
 
       it 'returns success' do
-        stub_request(:get, 'http://example.com/rest/api/2/serverInfo')
+        stub_jira_service_test
-          .to_return(status: 200, body: '{}')
 
         expect(Gitlab::HTTP).to receive(:get).with('/rest/api/2/serverInfo', any_args).and_call_original
 
@@ -200,6 +200,7 @@ RSpec.describe Projects::ServicesController do
 
     describe 'as JSON' do
       before do
+        stub_jira_service_test
         put :update, params: project_params(service: service_params, format: :json)
       end
 

spec/controllers/sessions_controller_spec.rb

@@ -229,7 +229,7 @@ RSpec.describe SessionsController do
 
           context 'when there are more than 5 anonymous session with the same IP' do
             before do
-              allow(Gitlab::AnonymousSession).to receive_message_chain(:new, :stored_sessions).and_return(6)
+              allow(Gitlab::AnonymousSession).to receive_message_chain(:new, :session_count).and_return(6)
             end
 
             it 'displays an error when the reCAPTCHA is not solved' do
@@ -241,7 +241,7 @@ RSpec.describe SessionsController do
             end
 
             it 'successfully logs in a user when reCAPTCHA is solved' do
-              expect(Gitlab::AnonymousSession).to receive_message_chain(:new, :cleanup_session_per_ip_entries)
+              expect(Gitlab::AnonymousSession).to receive_message_chain(:new, :cleanup_session_per_ip_count)
 
               succesful_login(user_params)
 

spec/features/projects/services/user_activates_jira_spec.rb

@@ -61,7 +61,10 @@ RSpec.describe 'User activates Jira', :js do
   end
 
   describe 'user disables the Jira Service' do
+    include JiraServiceHelper
+
     before do
+      stub_jira_service_test
       visit_project_integration('Jira')
       fill_form(disable: true)
       click_button('Save changes')

spec/frontend/alert_management/components/alert_details_spec.js

@@ -233,7 +233,7 @@ describe('AlertDetails', () => {
 
     describe('header', () => {
       const findHeader = () => wrapper.find('[data-testid="alert-header"]');
-      const stubs = { TimeAgoTooltip: '<span>now</span>' };
+      const stubs = { TimeAgoTooltip: { template: '<span>now</span>' } };
 
       describe('individual header fields', () => {
         describe.each`

spec/frontend/blob/suggest_gitlab_ci_yml/components/popover_spec.js

@@ -31,7 +31,7 @@ describe('Suggest gitlab-ci.yml Popover', () => {
         humanAccess,
       },
       stubs: {
-        'gl-popover': '<div><slot name="title"></slot><slot></slot></div>',
+        'gl-popover': { template: '<div><slot name="title"></slot><slot></slot></div>' },
       },
     });
   }

spec/frontend/diffs/components/commit_item_spec.js

@@ -24,8 +24,7 @@ describe('diffs/components/commit_item', () => {
 
   const getTitleElement = () => wrapper.find('.commit-row-message.item-title');
   const getDescElement = () => wrapper.find('pre.commit-row-description');
-  const getDescExpandElement = () =>
+  const getDescExpandElement = () => wrapper.find('.commit-content .js-toggle-button');
-    wrapper.find('.commit-content .text-expander.js-toggle-button');
   const getShaElement = () => wrapper.find('.commit-sha-group');
   const getAvatarElement = () => wrapper.find('.user-avatar-link');
   const getCommitterElement = () => wrapper.find('.committer');

spec/frontend/monitoring/components/embeds/embed_group_spec.js

@@ -71,14 +71,14 @@ describe('Embed Group', () => {
 
     it('is expanded by default', () => {
       metricsWithDataGetter.mockReturnValue([1]);
-      mountComponent({ shallow: false, stubs: { MetricEmbed: '<div />' } });
+      mountComponent({ shallow: false, stubs: { MetricEmbed: true } });
 
       expect(wrapper.find('.card-body').classes()).not.toContain('d-none');
     });
 
     it('collapses when clicked', done => {
       metricsWithDataGetter.mockReturnValue([1]);
-      mountComponent({ shallow: false, stubs: { MetricEmbed: '<div />' } });
+      mountComponent({ shallow: false, stubs: { MetricEmbed: true } });
 
       wrapper.find(GlButton).trigger('click');
 
@@ -148,14 +148,14 @@ describe('Embed Group', () => {
   describe('button text', () => {
     it('has a singular label when there is one embed', () => {
       metricsWithDataGetter.mockReturnValue([1]);
-      mountComponent({ shallow: false, stubs: { MetricEmbed: '<div />' } });
+      mountComponent({ shallow: false, stubs: { MetricEmbed: true } });
 
       expect(wrapper.find(GlButton).text()).toBe('Hide chart');
     });
 
     it('has a plural label when there are multiple embeds', () => {
       metricsWithDataGetter.mockReturnValue([2]);
-      mountComponent({ shallow: false, stubs: { MetricEmbed: '<div />' } });
+      mountComponent({ shallow: false, stubs: { MetricEmbed: true } });
 
       expect(wrapper.find(GlButton).text()).toBe('Hide charts');
     });

spec/lib/gitlab/anonymous_session_spec.rb

@@ -8,45 +8,36 @@ RSpec.describe Gitlab::AnonymousSession, :clean_gitlab_redis_shared_state do
 
   subject { new_anonymous_session }
 
-  def new_anonymous_session(session_id = default_session_id)
+  def new_anonymous_session
-    described_class.new('127.0.0.1', session_id: session_id)
+    described_class.new('127.0.0.1')
   end
 
-  describe '#store_session_id_per_ip' do
+  describe '#store_session_ip' do
     it 'adds session id to proper key' do
-      subject.store_session_id_per_ip
+      subject.count_session_ip
 
       Gitlab::Redis::SharedState.with do |redis|
-        expect(redis.smembers("session:lookup:ip:gitlab:127.0.0.1")).to eq [default_session_id]
+        expect(redis.get("session:lookup:ip:gitlab2:127.0.0.1").to_i).to eq 1
       end
     end
 
     it 'adds expiration time to key' do
       Timecop.freeze do
-        subject.store_session_id_per_ip
+        subject.count_session_ip
 
         Gitlab::Redis::SharedState.with do |redis|
-          expect(redis.ttl("session:lookup:ip:gitlab:127.0.0.1")).to eq(24.hours.to_i)
+          expect(redis.ttl("session:lookup:ip:gitlab2:127.0.0.1")).to eq(24.hours.to_i)
         end
       end
     end
 
-    it 'adds id only once' do
-      subject.store_session_id_per_ip
-      subject.store_session_id_per_ip
-
-      Gitlab::Redis::SharedState.with do |redis|
-        expect(redis.smembers("session:lookup:ip:gitlab:127.0.0.1")).to eq [default_session_id]
-      end
-    end
-
     context 'when there is already one session' do
-      it 'adds session id to proper key' do
+      it 'increments the session count' do
-        subject.store_session_id_per_ip
+        subject.count_session_ip
-        new_anonymous_session(additional_session_id).store_session_id_per_ip
+        new_anonymous_session.count_session_ip
 
         Gitlab::Redis::SharedState.with do |redis|
-          expect(redis.smembers("session:lookup:ip:gitlab:127.0.0.1")).to contain_exactly(default_session_id, additional_session_id)
+          expect(redis.get("session:lookup:ip:gitlab2:127.0.0.1").to_i).to eq(2)
         end
       end
     end
@@ -55,24 +46,22 @@ RSpec.describe Gitlab::AnonymousSession, :clean_gitlab_redis_shared_state do
   describe '#stored_sessions' do
     it 'returns all anonymous sessions per ip' do
       Gitlab::Redis::SharedState.with do |redis|
-        redis.sadd("session:lookup:ip:gitlab:127.0.0.1", default_session_id)
+        redis.set("session:lookup:ip:gitlab2:127.0.0.1", 2)
-        redis.sadd("session:lookup:ip:gitlab:127.0.0.1", additional_session_id)
       end
 
-      expect(subject.stored_sessions).to eq(2)
+      expect(subject.session_count).to eq(2)
     end
   end
 
   it 'removes obsolete lookup through ip entries' do
     Gitlab::Redis::SharedState.with do |redis|
-      redis.sadd("session:lookup:ip:gitlab:127.0.0.1", default_session_id)
+      redis.set("session:lookup:ip:gitlab2:127.0.0.1", 2)
-      redis.sadd("session:lookup:ip:gitlab:127.0.0.1", additional_session_id)
     end
 
-    subject.cleanup_session_per_ip_entries
+    subject.cleanup_session_per_ip_count
 
     Gitlab::Redis::SharedState.with do |redis|
-      expect(redis.smembers("session:lookup:ip:gitlab:127.0.0.1")).to eq [additional_session_id]
+      expect(redis.exists("session:lookup:ip:gitlab2:127.0.0.1")).to eq(false)
     end
   end
 end

spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb

@@ -3,15 +3,17 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::UsageDataCounters::EditorUniqueCounter, :clean_gitlab_redis_shared_state do
+  let(:user1) { build(:user, id: 1) }
+  let(:user2) { build(:user, id: 2) }
+  let(:user3) { build(:user, id: 3) }
+  let(:time) { Time.zone.now }
+
   shared_examples 'tracks and counts action' do
-    let(:user1) { build(:user, id: 1) }
+    before do
-    let(:user2) { build(:user, id: 2) }
+      stub_application_setting(usage_ping_enabled: true)
-    let(:user3) { build(:user, id: 3) }
+    end
-    let(:time) { Time.zone.now }
 
     specify do
-      stub_application_setting(usage_ping_enabled: true)
-
       aggregate_failures do
         expect(track_action(author: user1)).to be_truthy
         expect(track_action(author: user1)).to be_truthy
@@ -23,6 +25,10 @@ RSpec.describe Gitlab::UsageDataCounters::EditorUniqueCounter, :clean_gitlab_red
       end
     end
 
+    it 'does not track edit actions if author is not present' do
+      expect(track_action(author: nil)).to be_nil
+    end
+
     context 'when feature flag track_editor_edit_actions is disabled' do
       it 'does not track edit actions' do
         stub_feature_flags(track_editor_edit_actions: false)
@@ -67,4 +73,17 @@ RSpec.describe Gitlab::UsageDataCounters::EditorUniqueCounter, :clean_gitlab_red
       end
     end
   end
+
+  it 'can return the count of actions per user deduplicated ' do
+    described_class.track_web_ide_edit_action(author: user1)
+    described_class.track_snippet_editor_edit_action(author: user1)
+    described_class.track_sfe_edit_action(author: user1)
+    described_class.track_web_ide_edit_action(author: user2, time: time - 2.days)
+    described_class.track_web_ide_edit_action(author: user3, time: time - 3.days)
+    described_class.track_snippet_editor_edit_action(author: user3, time: time - 3.days)
+    described_class.track_sfe_edit_action(author: user3, time: time - 3.days)
+
+    expect(described_class.count_edit_using_editor(date_from: time, date_to: Date.today)).to eq(1)
+    expect(described_class.count_edit_using_editor(date_from: time - 5.days, date_to: Date.tomorrow)).to eq(3)
+  end
 end

spec/models/note_spec.rb

@@ -1416,4 +1416,20 @@ RSpec.describe Note do
       expect(note.parent_user).to be_nil
     end
   end
+
+  describe '#skip_notification?' do
+    subject(:skip_notification?) { note.skip_notification? }
+
+    context 'when there is no review' do
+      let(:note) { build(:note) }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'when the review exists' do
+      let(:note) { build(:note, :with_review) }
+
+      it { is_expected.to be_truthy }
+    end
+  end
 end

spec/models/project_services/jira_service_spec.rb

@@ -10,6 +10,11 @@ RSpec.describe JiraService do
   let(:username) { 'jira-username' }
   let(:password) { 'jira-password' }
   let(:transition_id) { 'test27' }
+  let(:server_info_results) { { 'deploymentType' => 'Cloud' } }
+
+  before do
+    WebMock.stub_request(:get, /serverInfo/).to_return(body: server_info_results.to_json )
+  end
 
   describe '#options' do
     let(:options) do
@@ -103,7 +108,7 @@ RSpec.describe JiraService do
       expect(subject.properties).to be_nil
     end
 
-    it 'stores data in data_fields correcty' do
+    it 'stores data in data_fields correctly' do
       service = subject
 
       expect(service.jira_tracker_data.url).to eq(url)
@@ -111,6 +116,35 @@ RSpec.describe JiraService do
       expect(service.jira_tracker_data.username).to eq(username)
       expect(service.jira_tracker_data.password).to eq(password)
       expect(service.jira_tracker_data.jira_issue_transition_id).to eq(transition_id)
+      expect(service.jira_tracker_data.deployment_cloud?).to be_truthy
+    end
+
+    context 'when loading serverInfo' do
+      let!(:jira_service) { subject }
+
+      context 'Cloud instance' do
+        let(:server_info_results) { { 'deploymentType' => 'Cloud' } }
+
+        it 'is detected' do
+          expect(jira_service.jira_tracker_data.deployment_cloud?).to be_truthy
+        end
+      end
+
+      context 'Server instance' do
+        let(:server_info_results) { { 'deploymentType' => 'Server' } }
+
+        it 'is detected' do
+          expect(jira_service.jira_tracker_data.deployment_server?).to be_truthy
+        end
+      end
+
+      context 'Unknown instance' do
+        let(:server_info_results) { { 'deploymentType' => 'FutureCloud' } }
+
+        it 'is detected' do
+          expect(jira_service.jira_tracker_data.deployment_unknown?).to be_truthy
+        end
+      end
     end
   end
 
@@ -151,8 +185,8 @@ RSpec.describe JiraService do
 
       describe '#update' do
         context 'basic update' do
-          let(:new_username) { 'new_username' }
+          let_it_be(:new_username) { 'new_username' }
-          let(:new_url) { 'http://jira-new.example.com' }
+          let_it_be(:new_url) { 'http://jira-new.example.com' }
 
           before do
             service.update(username: new_username, url: new_url)
@@ -173,6 +207,53 @@ RSpec.describe JiraService do
           end
         end
 
+        context 'when updating the url, api_url, username, or password' do
+          it 'updates deployment type' do
+            service.update(url: 'http://first.url')
+            service.jira_tracker_data.update(deployment_type: 'server')
+
+            expect(service.jira_tracker_data.deployment_server?).to be_truthy
+
+            service.update(api_url: 'http://another.url')
+            service.jira_tracker_data.reload
+
+            expect(service.jira_tracker_data.deployment_cloud?).to be_truthy
+            expect(WebMock).to have_requested(:get, /serverInfo/).twice
+          end
+
+          it 'calls serverInfo for url' do
+            service.update(url: 'http://first.url')
+
+            expect(WebMock).to have_requested(:get, /serverInfo/)
+          end
+
+          it 'calls serverInfo for api_url' do
+            service.update(api_url: 'http://another.url')
+
+            expect(WebMock).to have_requested(:get, /serverInfo/)
+          end
+
+          it 'calls serverInfo for username' do
+            service.update(username: 'test-user')
+
+            expect(WebMock).to have_requested(:get, /serverInfo/)
+          end
+
+          it 'calls serverInfo for password' do
+            service.update(password: 'test-password')
+
+            expect(WebMock).to have_requested(:get, /serverInfo/)
+          end
+        end
+
+        context 'when not updating the url, api_url, username, or password' do
+          it 'does not update deployment type' do
+            service.update(jira_issue_transition_id: 'jira_issue_transition_id')
+
+            expect(WebMock).not_to have_requested(:get, /serverInfo/)
+          end
+        end
+
         context 'stored password invalidation' do
           context 'when a password was previously set' do
             context 'when only web url present' do
@@ -627,6 +708,7 @@ RSpec.describe JiraService do
   end
 
   describe '#test' do
+    let(:server_info_results) { { 'url' => 'http://url', 'deploymentType' => 'Cloud' } }
     let(:jira_service) do
       described_class.new(
         url: url,
@@ -635,24 +717,21 @@ RSpec.describe JiraService do
       )
     end
 
-    def test_settings(url = 'jira.example.com')
+    def server_info
-      test_url = "http://#{url}/rest/api/2/serverInfo"
-
-      WebMock.stub_request(:get, test_url).with(basic_auth: [username, password])
-        .to_return(body: { url: 'http://url' }.to_json )
-
       jira_service.test(nil)
     end
 
     context 'when the test succeeds' do
       it 'gets Jira project with URL when API URL not set' do
-        expect(test_settings).to eq(success: true, result: { 'url' => 'http://url' })
+        expect(server_info).to eq(success: true, result: server_info_results)
+        expect(WebMock).to have_requested(:get, /jira.example.com/)
       end
 
       it 'gets Jira project with API URL if set' do
         jira_service.update(api_url: 'http://jira.api.com')
 
-        expect(test_settings('jira.api.com')).to eq(success: true, result: { 'url' => 'http://url' })
+        expect(server_info).to eq(success: true, result: server_info_results)
+        expect(WebMock).to have_requested(:get, /jira.api.com/)
       end
     end
 

spec/requests/api/ci/runner/jobs_artifacts_spec.rb

@@ -143,6 +143,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
                 expect(response.media_type).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
                 expect(json_response['TempPath']).to eq(JobArtifactUploader.workhorse_local_upload_path)
                 expect(json_response['RemoteObject']).to be_nil
+                expect(json_response['MaximumSize']).not_to be_nil
               end
             end
 
@@ -167,6 +168,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
                   expect(json_response['RemoteObject']).to have_key('StoreURL')
                   expect(json_response['RemoteObject']).to have_key('DeleteURL')
                   expect(json_response['RemoteObject']).to have_key('MultipartUpload')
+                  expect(json_response['MaximumSize']).not_to be_nil
                 end
               end
 
@@ -188,6 +190,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do
             expect(response).to have_gitlab_http_status(:ok)
             expect(response.media_type).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
             expect(json_response['TempPath']).not_to be_nil
+            expect(json_response['MaximumSize']).not_to be_nil
           end
 
           it 'fails to post too large artifact' do

spec/requests/api/graphql/project/releases_spec.rb

@@ -14,6 +14,7 @@ RSpec.describe 'Query.project(fullPath).releases()' do
     graphql_query_for(:project, { fullPath: project.full_path },
     %{
       releases {
+        count
         nodes {
           tagName
           tagPath
@@ -53,6 +54,20 @@ RSpec.describe 'Query.project(fullPath).releases()' do
     stub_default_url_options(host: 'www.example.com')
   end
 
+  shared_examples 'correct total count' do
+    let(:data) { graphql_data.dig('project', 'releases') }
+
+    before do
+      create_list(:release, 2, project: project)
+
+      post_query
+    end
+
+    it 'returns the total count' do
+      expect(data['count']).to eq(project.releases.count)
+    end
+  end
+
   shared_examples 'full access to all repository-related fields' do
     describe 'repository-related fields' do
       before do
@@ -92,6 +107,8 @@ RSpec.describe 'Query.project(fullPath).releases()' do
         )
       end
     end
+
+    it_behaves_like 'correct total count'
   end
 
   shared_examples 'no access to any repository-related fields' do
@@ -119,6 +136,8 @@ RSpec.describe 'Query.project(fullPath).releases()' do
         )
       end
     end
+
+    it_behaves_like 'correct total count'
   end
 
   # editUrl is tested separately becuase its permissions

spec/rubocop/cop/migration/refer_to_index_by_name_spec.rb

@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+#
+require 'fast_spec_helper'
+require 'rubocop'
+require_relative '../../../../rubocop/cop/migration/refer_to_index_by_name'
+
+RSpec.describe RuboCop::Cop::Migration::ReferToIndexByName, type: :rubocop do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  context 'in migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(true)
+    end
+
+    context 'when existing indexes are referred to without an explicit name' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class TestReferToIndexByName < ActiveRecord::Migration[6.0]
+            DOWNTIME = false
+
+            INDEX_NAME = 'my_test_name'
+
+            disable_ddl_transaction!
+
+            def up
+              if index_exists? :test_indexes, :column1, name: 'index_name_1'
+                remove_index :test_indexes, column: :column1, name: 'index_name_1'
+              end
+
+              if index_exists? :test_indexes, :column2
+                 ^^^^^^^^^^^^^ #{described_class::MSG}
+                remove_index :test_indexes, :column2
+                ^^^^^^^^^^^^ #{described_class::MSG}
+              end
+
+              remove_index :test_indexes, column: column3
+              ^^^^^^^^^^^^ #{described_class::MSG}
+
+              remove_index :test_indexes, name: 'index_name_4'
+            end
+
+            def down
+              if index_exists? :test_indexes, :column4, using: :gin, opclass: :gin_trgm_ops
+                 ^^^^^^^^^^^^^ #{described_class::MSG}
+                remove_concurrent_index :test_indexes, :column4, using: :gin, opclass: :gin_trgm_ops
+                ^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
+              end
+
+              if index_exists? :test_indexes, :column3, unique: true, name: 'index_name_3', where: 'column3 = 10'
+                remove_concurrent_index :test_indexes, :column3, unique: true, name: 'index_name_3', where: 'column3 = 10'
+              end
+            end
+          end
+        RUBY
+
+        expect(cop.offenses.map(&:cop_name)).to all(eq("Migration/#{described_class.name.demodulize}"))
+      end
+    end
+  end
+
+  context 'outside migration' do
+    before do
+      allow(cop).to receive(:in_migration?).and_return(false)
+    end
+
+    it 'registers no offenses' do
+      expect_no_offenses(<<~RUBY)
+        class TestReferToIndexByName < ActiveRecord::Migration[6.0]
+          DOWNTIME = false
+
+          disable_ddl_transaction!
+
+          def up
+            if index_exists? :test_indexes, :column1
+              remove_index :test_indexes, :column1
+            end
+          end
+
+          def down
+            if index_exists? :test_indexes, :column1
+              remove_concurrent_index :test_indexes, :column1
+            end
+          end
+        end
+      RUBY
+    end
+  end
+end

spec/services/admin/propagate_integration_service_spec.rb

@@ -4,6 +4,12 @@ require 'spec_helper'
 
 RSpec.describe Admin::PropagateIntegrationService do
   describe '.propagate' do
+    include JiraServiceHelper
+
+    before do
+      stub_jira_service_test
+    end
+
     let(:excluded_attributes) { %w[id project_id inherit_from_id instance created_at updated_at default] }
     let!(:project) { create(:project) }
     let!(:instance_integration) do

spec/services/git/branch_push_service_spec.rb

@@ -416,6 +416,7 @@ RSpec.describe Git::BranchPushService, services: true do
       before do
         # project.create_jira_service doesn't seem to invalidate the cache here
         project.has_external_issue_tracker = true
+        stub_jira_service_test
         jira_service_settings
         stub_jira_urls("JIRA-1")
 

spec/services/merge_requests/merge_service_spec.rb

@@ -152,6 +152,7 @@ RSpec.describe MergeRequests::MergeService do
         let(:commit)       { double('commit', safe_message: "Fixes #{jira_issue.to_reference}") }
 
         before do
+          stub_jira_service_test
           project.update!(has_external_issue_tracker: true)
           jira_service_settings
           stub_jira_urls(jira_issue.id)

spec/services/projects/propagate_service_template_spec.rb

@@ -79,7 +79,11 @@ RSpec.describe Projects::PropagateServiceTemplate do
     end
 
     context 'service with data fields' do
+      include JiraServiceHelper
+
       let(:service_template) do
+        stub_jira_service_test
+
         JiraService.create!(
           template: true,
           active: true,

spec/services/system_note_service_spec.rb

@@ -347,6 +347,7 @@ RSpec.describe SystemNoteService do
     let(:success_message) { "SUCCESS: Successfully posted to http://jira.example.net." }
 
     before do
+      stub_jira_service_test
       stub_jira_urls(jira_issue.id)
       jira_service_settings
     end

spec/support/helpers/jira_service_helper.rb

@@ -78,8 +78,7 @@ module JiraServiceHelper
   end
 
   def stub_jira_service_test
-    WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/serverInfo')
+    WebMock.stub_request(:get, /serverInfo/).to_return(body: { url: 'http://url' }.to_json)
-      .to_return(body: { url: 'http://url' }.to_json)
   end
 
   def stub_jira_urls(issue_id)

spec/support/shared_contexts/services_shared_context.rb

@@ -2,6 +2,8 @@
 
 Service.available_services_names.each do |service|
   RSpec.shared_context service do
+    include JiraServiceHelper if service == 'jira'
+
     let(:dashed_service) { service.dasherize }
     let(:service_method) { "#{service}_service".to_sym }
     let(:service_klass) { "#{service}_service".classify.constantize }
@@ -39,6 +41,7 @@ Service.available_services_names.each do |service|
 
     before do
       enable_license_for_service(service)
+      stub_jira_service_test if service == 'jira'
     end
 
     def initialize_service(service)

spec/uploaders/object_storage_spec.rb

@@ -414,28 +414,38 @@ RSpec.describe ObjectStorage do
 
     subject { uploader_class.workhorse_authorize(has_length: has_length, maximum_size: maximum_size) }
 
-    shared_examples 'uses local storage' do
+    shared_examples 'returns the maximum size given' do
       it "returns temporary path" do
-        is_expected.to have_key(:TempPath)
+        expect(subject[:MaximumSize]).to eq(maximum_size)
+      end
+    end
 
-        expect(subject[:TempPath]).to start_with(uploader_class.root)
+    shared_examples 'uses local storage' do
-        expect(subject[:TempPath]).to include(described_class::TMP_UPLOAD_PATH)
+      it_behaves_like 'returns the maximum size given' do
+        it "returns temporary path" do
+          is_expected.to have_key(:TempPath)
+
+          expect(subject[:TempPath]).to start_with(uploader_class.root)
+          expect(subject[:TempPath]).to include(described_class::TMP_UPLOAD_PATH)
+        end
       end
     end
 
     shared_examples 'uses remote storage' do
-      it "returns remote store" do
+      it_behaves_like 'returns the maximum size given' do
-        is_expected.to have_key(:RemoteObject)
+        it "returns remote store" do
+          is_expected.to have_key(:RemoteObject)
 
-        expect(subject[:RemoteObject]).to have_key(:ID)
+          expect(subject[:RemoteObject]).to have_key(:ID)
-        expect(subject[:RemoteObject]).to include(Timeout: a_kind_of(Integer))
+          expect(subject[:RemoteObject]).to include(Timeout: a_kind_of(Integer))
-        expect(subject[:RemoteObject][:Timeout]).to be(ObjectStorage::DirectUpload::TIMEOUT)
+          expect(subject[:RemoteObject][:Timeout]).to be(ObjectStorage::DirectUpload::TIMEOUT)
-        expect(subject[:RemoteObject]).to have_key(:GetURL)
+          expect(subject[:RemoteObject]).to have_key(:GetURL)
-        expect(subject[:RemoteObject]).to have_key(:DeleteURL)
+          expect(subject[:RemoteObject]).to have_key(:DeleteURL)
-        expect(subject[:RemoteObject]).to have_key(:StoreURL)
+          expect(subject[:RemoteObject]).to have_key(:StoreURL)
-        expect(subject[:RemoteObject][:GetURL]).to include(described_class::TMP_UPLOAD_PATH)
+          expect(subject[:RemoteObject][:GetURL]).to include(described_class::TMP_UPLOAD_PATH)
-        expect(subject[:RemoteObject][:DeleteURL]).to include(described_class::TMP_UPLOAD_PATH)
+          expect(subject[:RemoteObject][:DeleteURL]).to include(described_class::TMP_UPLOAD_PATH)
-        expect(subject[:RemoteObject][:StoreURL]).to include(described_class::TMP_UPLOAD_PATH)
+          expect(subject[:RemoteObject][:StoreURL]).to include(described_class::TMP_UPLOAD_PATH)
+        end
       end
     end
 

spec/workers/new_note_worker_spec.rb

@@ -50,10 +50,20 @@ RSpec.describe NewNoteWorker do
     end
   end
 
-  context 'when note is with review' do
+  context 'when note does not require notification' do
-    it 'does not create a new note notification' do
+    let(:note) { create(:note) }
-      note = create(:note, :with_review)
 
+    before do
+      # TODO: `allow_next_instance_of` helper method is not working
+      # because ActiveRecord is directly calling `.allocate` on model
+      # classes and bypasses the `.new` method call.
+      # Fix the `allow_next_instance_of` helper and change these to mock
+      # the next instance of `Note` model class.
+      allow(Note).to receive(:find_by).with(id: note.id).and_return(note)
+      allow(note).to receive(:skip_notification?).and_return(true)
+    end
+
+    it 'does not create a new note notification' do
       expect_any_instance_of(NotificationService).not_to receive(:new_note)
 
       subject.perform(note.id)