Automatic merge of gitlab-org/gitlab master

Gemfile

@@ -113,7 +113,7 @@ gem 'carrierwave', '~> 1.3'
 gem 'mini_magick', '~> 4.10.1'
 
 # for backups
-gem 'fog-aws', '~> 3.7'
+gem 'fog-aws', '~> 3.8'
 # Locked until fog-google resolves https://github.com/fog/fog-google/issues/421.
 # Also see config/initializers/fog_core_patch.rb.
 gem 'fog-core', '= 2.1.0'

Gemfile.lock

@@ -360,7 +360,7 @@ GEM
       fog-json
       ipaddress (~> 0.8)
       xml-simple (~> 1.1)
-    fog-aws (3.7.0)
+    fog-aws (3.8.0)
       fog-core (~> 2.1)
       fog-json (~> 1.1)
       fog-xml (~> 0.1)
@@ -1346,7 +1346,7 @@ DEPENDENCIES
   flipper-active_support_cache_store (~> 0.17.1)
   flowdock (~> 0.7)
   fog-aliyun (~> 0.3)
-  fog-aws (~> 3.7)
+  fog-aws (~> 3.8)
   fog-core (= 2.1.0)
   fog-google (~> 1.12)
   fog-local (~> 0.6)

app/assets/javascripts/boards/boards_util.js

 import { sortBy } from 'lodash';
+import { __ } from '~/locale';
 import { ListType } from './constants';
 import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 
@@ -8,14 +9,15 @@ export function getMilestone() {
 
 export function updateListPosition(listObj) {
   const { listType } = listObj;
-  let { position } = listObj;
+  let { position, title } = listObj;
   if (listType === ListType.closed) {
     position = Infinity;
   } else if (listType === ListType.backlog) {
     position = -Infinity;
+    title = __('Open');
   }
 
-  return { ...listObj, position };
+  return { ...listObj, title, position };
 }
 
 export function formatBoardLists(lists) {

app/assets/javascripts/boards/stores/actions.js

@@ -12,6 +12,7 @@ import {
   fullBoardId,
   formatListsPageInfo,
   formatIssue,
+  updateListPosition,
 } from '../boards_util';
 import createFlash from '~/flash';
 import { __ } from '~/locale';
@@ -131,7 +132,7 @@ export default {
   },
 
   addList: ({ commit }, list) => {
-    commit(types.RECEIVE_ADD_LIST_SUCCESS, list);
+    commit(types.RECEIVE_ADD_LIST_SUCCESS, updateListPosition(list));
   },
 
   fetchLabels: ({ state, commit }, searchTerm) => {

app/graphql/types/base_enum.rb

@@ -5,6 +5,25 @@ module Types
     extend GitlabStyleDeprecations
 
     class << self
+      # Registers enum definition by the given DeclarativeEnum module
+      #
+      # @param enum_mod [Module] The enum module to be used
+      # @param use_name [Boolean] Does not override the name if set `false`
+      # @param use_description [Boolean] Does not override the description if set `false`
+      #
+      # Example:
+      #
+      #   class MyEnum < BaseEnum
+      #     declarative_enum MyDeclarativeEnum
+      #   end
+      #
+      def declarative_enum(enum_mod, use_name: true, use_description: true)
+        graphql_name(enum_mod.name) if use_name
+        description(enum_mod.description) if use_description
+
+        enum_mod.definition.each { |key, content| value(key.to_s.upcase, content) }
+      end
+
       def value(*args, **kwargs, &block)
         enum[args[0].downcase] = kwargs[:value] || args[0]
         kwargs = gitlab_deprecation(kwargs)

app/models/application_record.rb

@@ -77,4 +77,9 @@ class ApplicationRecord < ActiveRecord::Base
   def self.where_exists(query)
     where('EXISTS (?)', query.select(1))
   end
+
+  def self.declarative_enum(enum_mod)
+    values = enum_mod.definition.transform_values { |v| v[:value] }
+    enum(enum_mod.key => values)
+  end
 end

changelogs/unreleased/georgekoltsov-fix-bulk-import-graphql-extractor.yml

+---
+title: Update GraphqlExtractor return value to be original hash
+merge_request: 51596
+author:
+type: fixed

config/initializers/carrierwave_patch.rb

@@ -23,7 +23,9 @@ module CarrierWave
           # Multithreaded uploads are essential for copying large amounts of data
           # within the request timeout.
           if ::Feature.enabled?(:s3_multithreaded_uploads, default_enabled: true) && fog_provider == 'AWS'
-            file.concurrency = 10 # AWS SDK uses 10 threads by default
+            # AWS SDK uses 10 threads by default and a multipart chunk size of 10 MB
+            file.concurrency = 10
+            file.multipart_chunk_size = 10485760
             file.copy(@uploader.fog_directory, new_path, copy_to_options)
           else
             # Some Fog providers may issue a GET request (https://github.com/fog/fog-google/issues/512)

ee/app/enums/vulnerabilities/dismissal_reason_enum.rb

+# frozen_string_literal: true
+
+module Vulnerabilities
+  module DismissalReasonEnum
+    extend DeclarativeEnum
+
+    key :dismissal_reason
+    name 'VulnerabilityDismissalReason'
+    description 'The dismissal reason of the Vulnerability'
+
+    define do
+      acceptable_risk value: 0, description: 'The likelihood of the Vulnerability occurring and its impact are deemed acceptable'
+      false_positive value: 1, description: 'The Vulnerability was incorrectly identified as being present'
+      mitigating_control value: 2, description: 'There is a mitigating control that eliminates the Vulnerability or makes its risk acceptable'
+      used_in_tests value: 3, description: 'The Vulnerability is used in tests and does not pose an actual risk'
+      not_applicable value: 4, description: 'Other reasons for dismissal'
+    end
+  end
+end

ee/app/graphql/types/vulnerabilities/dismissal_reason_enum.rb

@@ -3,20 +3,7 @@
 module Types
   module Vulnerabilities
     class DismissalReasonEnum < BaseEnum
-      graphql_name 'VulnerabilityDismissalReason'
-      description 'The dismissal reason of the Vulnerability'
-
-      DISMISSAL_DESCRIPTIONS = {
-        acceptable_risk: 'The likelihood of the Vulnerability occurring and its impact are deemed acceptable',
-        false_positive: 'The Vulnerability was incorrectly identified as being present',
-        mitigating_control: 'There is a mitigating control that eliminates the Vulnerability or makes its risk acceptable',
-        used_in_tests: 'The Vulnerability is used in tests and does not pose an actual risk',
-        not_applicable: 'Other reasons for dismissal'
-      }.freeze
-
-      ::Vulnerabilities::Feedback.dismissal_reasons.keys.each do |dismissal_reason|
-        value dismissal_reason.to_s.upcase, value: dismissal_reason.to_s, description: DISMISSAL_DESCRIPTIONS[dismissal_reason.to_sym]
-      end
+      declarative_enum ::Vulnerabilities::DismissalReasonEnum
     end
   end
 end

ee/app/models/vulnerabilities/feedback.rb

@@ -14,9 +14,9 @@ module Vulnerabilities
 
     attr_accessor :vulnerability_data
 
-    enum dismissal_reason: { acceptable_risk: 0, false_positive: 1, mitigating_control: 2, used_in_tests: 3, not_applicable: 4 }
     enum feedback_type: { dismissal: 0, issue: 1, merge_request: 2 }, _prefix: :for
     enum category: ::Enums::Vulnerability.report_types
+    declarative_enum DismissalReasonEnum
 
     validates :project, presence: true
     validates :author, presence: true

ee/changelogs/unreleased/296976-the-open-list-in-boards-with-swimlanes-applied-is-mislabeled-as.yml

+---
+title: Rename Backlog list to Open for Swimlanes
+merge_request: 51539
+author:
+type: fixed

ee/spec/features/boards/swimlanes/epics_swimlanes_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe 'epics swimlanes', :js do
   let_it_be(:project) { create(:project, :public, group: group) }
 
   let_it_be(:board) { create(:board, project: project) }
-  let_it_be(:label) { create(:label, project: project, name: 'Label 1') }
+  let_it_be(:label) { create(:label, project: project, name: 'Label1') }
   let_it_be(:list) { create(:list, board: board, label: label, position: 0) }
 
   let_it_be(:issue1) { create(:issue, project: project, labels: [label]) }
@@ -40,6 +40,18 @@ RSpec.describe 'epics swimlanes', :js do
         expect(page.find('span[data-testid="issues-lane-issue-count"]')).to have_content('1')
       end
     end
+
+    it 'displays default lists and a label list' do
+      lists = %w[Open Label1 Closed]
+
+      wait_for_requests
+
+      expect(page).to have_selector('.board-header', count: 3)
+
+      page.all('.board-header').each_with_index do |list, i|
+        expect(list.find('.board-title')).to have_content(lists[i])
+      end
+    end
   end
 
   before do

lib/bulk_imports/common/extractors/graphql_extractor.rb

@@ -11,14 +11,10 @@ module BulkImports
         def extract(context)
           client = graphql_client(context)
 
-          Enumerator.new do |yielder|
-            result = client.execute(
+          client.execute(
             client.parse(query.to_s),
             query.variables(context.entity)
-            )
-
-            yielder << result.original_hash.deep_dup
-          end
+          ).original_hash.deep_dup
         end
 
         private

lib/declarative_enum.rb

+# frozen_string_literal: true
+
+# Extending this module will give you the ability of defining
+# enum values in a declarative way.
+#
+#   module DismissalReasons
+#     extend DeclarativeEnum
+#
+#     key  :dismissal_reason
+#     name 'DismissalReasonOfVulnerability'
+#
+#     description <<~TEXT
+#       This enum holds the user selected dismissal reason
+#       when they are dismissing the vulnerabilities
+#     TEXT
+#
+#     define do
+#       acceptable_risk value: 0, description: 'The vulnerability is known but is considered to be an acceptable business risk.'
+#       false_positive value: 1, description: 'An error in reporting the presence of a vulnerability in a system when the vulnerability is not present.'
+#       used_in_tests value: 2, description: 'The finding is not a vulnerability because it is part of a test or is test data.'
+#     end
+#
+# Then we can use this module to register enums for our Active Record models like so,
+#
+#   class VulnerabilityFeedback
+#     declarative_enum DismissalReasons
+#   end
+#
+# Also we can use this module to create GraphQL Enum types like so,
+#
+# module Types
+#   module Vulnerabilities
+#     class DismissalReasonEnum < BaseEnum
+#       declarative_enum DismissalReasons
+#     end
+#   end
+# end
+#
+# rubocop:disable Gitlab/ModuleWithInstanceVariables
+module DeclarativeEnum
+  # This `prepended` hook will merge the enum definition
+  # of the prepended module into the base module to be
+  # used by `prepend_if_ee` helper method.
+  def prepended(base)
+    base.definition.merge!(definition)
+  end
+
+  def key(new_key = nil)
+    @key = new_key if new_key
+
+    @key
+  end
+
+  def name(new_name = nil)
+    @name = new_name if new_name
+
+    @name
+  end
+
+  def description(new_description = nil)
+    @description = new_description if new_description
+
+    @description
+  end
+
+  def define(&block)
+    raise LocalJumpError.new('No block given') unless block
+
+    @definition = Builder.new(definition, block).build
+  end
+
+  # We can use this method later to apply some sanity checks
+  # but for now, returning a Hash without any check is enough.
+  def definition
+    @definition.to_h
+  end
+
+  class Builder
+    KeyCollisionError = Class.new(StandardError)
+
+    def initialize(definition, block)
+      @definition = definition
+      @block = block
+    end
+
+    def build
+      instance_exec(&@block)
+
+      @definition
+    end
+
+    private
+
+    def method_missing(name, *arguments, value: nil, description: nil, &block)
+      key = name.downcase.to_sym
+      raise KeyCollisionError, "'#{key}' collides with an existing enum key!" if @definition[key]
+
+      @definition[key] = {
+        value: value,
+        description: description
+      }
+    end
+  end
+end
+# rubocop:enable Gitlab/ModuleWithInstanceVariables

spec/graphql/types/base_enum_spec.rb

@@ -3,7 +3,75 @@
 require 'spec_helper'
 
 RSpec.describe Types::BaseEnum do
-  describe '#enum' do
+  describe '.declarative_enum' do
+    let(:use_name) { true }
+    let(:use_description) { true }
+    let(:enum_type) do
+      Class.new(described_class) do
+        graphql_name 'OriginalName'
+        description 'Original description'
+      end
+    end
+
+    let(:enum_module) do
+      Module.new do
+        extend DeclarativeEnum
+
+        name 'Name'
+        description 'Description'
+
+        define do
+          foo value: 0, description: 'description of foo'
+        end
+      end
+    end
+
+    subject(:set_declarative_enum) { enum_type.declarative_enum(enum_module, use_name: use_name, use_description: use_description) }
+
+    describe '#graphql_name' do
+      context 'when the use_name is `true`' do
+        it 'changes the graphql_name' do
+          expect { set_declarative_enum }.to change { enum_type.graphql_name }.from('OriginalName').to('Name')
+        end
+      end
+
+      context 'when the use_name is `false`' do
+        let(:use_name) { false }
+
+        it 'does not change the graphql_name' do
+          expect { set_declarative_enum }.not_to change { enum_type.graphql_name }.from('OriginalName')
+        end
+      end
+    end
+
+    describe '#description' do
+      context 'when the use_description is `true`' do
+        it 'changes the description' do
+          expect { set_declarative_enum }.to change { enum_type.description }.from('Original description').to('Description')
+        end
+      end
+
+      context 'when the use_description is `false`' do
+        let(:use_description) { false }
+
+        it 'does not change the description' do
+          expect { set_declarative_enum }.not_to change { enum_type.description }.from('Original description')
+        end
+      end
+    end
+
+    describe '#values' do
+      it 'sets the values defined by the declarative enum' do
+        set_declarative_enum
+
+        expect(enum_type.values.keys).to eq(['FOO'])
+        expect(enum_type.values.values.map(&:description)).to eq(['description of foo'])
+        expect(enum_type.values.values.map(&:value)).to eq([0])
+      end
+    end
+  end
+
+  describe '.enum' do
     let(:enum) do
       Class.new(described_class) do
         value 'TEST', value: 3

spec/initializers/carrierwave_patch_spec.rb

@@ -39,6 +39,14 @@ RSpec.describe 'CarrierWave::Storage::Fog::File' do
       let(:dest_filename) { 'copied.txt'}
 
       it 'copies the file' do
+        fog_file = subject.send(:file)
+
+        expect(fog_file).to receive(:concurrency=).with(10).and_call_original
+        # multipart_chunk_size must be explicitly set in order to leverage
+        # multithreaded, multipart transfers for files below 5GB.
+        expect(fog_file).to receive(:multipart_chunk_size=).with(10.megabytes).and_call_original
+        expect(fog_file).to receive(:copy).with(bucket_name, dest_filename, anything).and_call_original
+
         result = subject.copy_to(dest_filename)
 
         expect(result.exists?).to be true

spec/lib/bulk_imports/common/extractors/graphql_extractor_spec.rb

@@ -27,11 +27,8 @@ RSpec.describe BulkImports::Common::Extractors::GraphqlExtractor do
       allow(graphql_client).to receive(:execute).and_return(response)
     end
 
-    it 'returns an enumerator with fetched results' do
-      response = subject.extract(context)
-
-      expect(response).to be_instance_of(Enumerator)
-      expect(response.first).to eq({ foo: :bar })
+    it 'returns original hash' do
+      expect(subject.extract(context)).to eq({ foo: :bar })
     end
   end
 

spec/lib/declarative_enum_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DeclarativeEnum do
+  let(:enum_module) do
+    Module.new do
+      extend DeclarativeEnum
+
+      key :my_enum
+      name 'MyEnumName'
+
+      description "Enum description"
+
+      define do
+        foo value: 0, description: 'description of foo'
+        bar value: 1, description: 'description of bar'
+      end
+    end
+  end
+
+  let(:original_definition) do
+    {
+      foo: { description: 'description of foo', value: 0 },
+      bar: { description: 'description of bar', value: 1 }
+    }
+  end
+
+  describe '.key' do
+    subject(:key) { enum_module.key(new_key) }
+
+    context 'when the argument is set' do
+      let(:new_key) { :new_enum_key }
+
+      it 'changes the key' do
+        expect { key }.to change { enum_module.key }.from(:my_enum).to(:new_enum_key)
+      end
+    end
+
+    context 'when the argument is `nil`' do
+      let(:new_key) { nil }
+
+      it { is_expected.to eq(:my_enum) }
+    end
+  end
+
+  describe '.name' do
+    subject(:name) { enum_module.name(new_name) }
+
+    context 'when the argument is set' do
+      let(:new_name) { 'NewMyEnumName' }
+
+      it 'changes the name' do
+        expect { name }.to change { enum_module.name }.from('MyEnumName').to('NewMyEnumName')
+      end
+    end
+
+    context 'when the argument is `nil`' do
+      let(:new_name) { nil }
+
+      it { is_expected.to eq('MyEnumName') }
+    end
+  end
+
+  describe '.description' do
+    subject(:description) { enum_module.description(new_description) }
+
+    context 'when the argument is set' do
+      let(:new_description) { 'New enum description' }
+
+      it 'changes the description' do
+        expect { description }.to change { enum_module.description }.from('Enum description').to('New enum description')
+      end
+    end
+
+    context 'when the argument is `nil`' do
+      let(:new_description) { nil }
+
+      it { is_expected.to eq('Enum description') }
+    end
+  end
+
+  describe '.define' do
+    subject(:define) { enum_module.define(&block) }
+
+    context 'when there is a block given' do
+      context 'when the given block tries to register the same key' do
+        let(:block) do
+          proc do
+            foo value: 2, description: 'description of foo'
+          end
+        end
+
+        it 'raises a `KeyCollisionError`' do
+          expect { define }.to raise_error(DeclarativeEnum::Builder::KeyCollisionError)
+        end
+      end
+
+      context 'when the given block does not try to register the same key' do
+        let(:expected_new_definition) { original_definition.merge(zoo: { description: 'description of zoo', value: 0 }) }
+        let(:block) do
+          proc do
+            zoo value: 0, description: 'description of zoo'
+          end
+        end
+
+        it 'appends the new definition' do
+          expect { define }.to change { enum_module.definition }.from(original_definition).to(expected_new_definition)
+        end
+      end
+    end
+
+    context 'when there is no block given' do
+      let(:block) { nil }
+
+      it 'raises a LocalJumpError' do
+        expect { define }.to raise_error(LocalJumpError)
+      end
+    end
+  end
+
+  describe '.definition' do
+    subject { enum_module.definition }
+
+    it { is_expected.to eq(original_definition) }
+  end
+
+  describe 'extending the enum module' do
+    let(:extended_definition) { original_definition.merge(zoo: { value: 2, description: 'description of zoo' }) }
+    let(:new_enum_module) do
+      Module.new do
+        extend DeclarativeEnum
+
+        define do
+          zoo value: 2, description: 'description of zoo'
+        end
+      end
+    end
+
+    subject(:prepend_new_enum_module) { enum_module.prepend(new_enum_module) }
+
+    it 'extends the values of the base enum module' do
+      expect { prepend_new_enum_module }.to change { enum_module.definition }.from(original_definition)
+                                                                             .to(extended_definition)
+    end
+  end
+end

spec/services/merge_requests/export_csv_service_spec.rb

@@ -27,11 +27,11 @@ RSpec.describe MergeRequests::ExportCsvService do
         let_it_be(:merge_request) { create(:merge_request, assignees: create_list(:user, 2)) }
 
         it 'contains the names of assignees' do
-          expect(csv['Assignees']).to eq(merge_request.assignees.map(&:name).join(', '))
+          expect(csv['Assignees'].split(', ')).to match_array(merge_request.assignees.map(&:name))
         end
 
         it 'contains the usernames of assignees' do
-          expect(csv['Assignee Usernames']).to eq(merge_request.assignees.map(&:username).join(', '))
+          expect(csv['Assignee Usernames'].split(', ')).to match_array(merge_request.assignees.map(&:username))
         end
       end