Add latest changes from gitlab-org/gitlab@master

locale/gitlab.pot

@@ -3256,6 +3256,9 @@ msgstr ""
 msgid "Can't find variable: ZiteReader"
 msgstr ""
+msgid "Can't load mermaid module: %{err}"
+msgstr ""
+
 msgid "Can't remove group members without group managed account"
 msgstr ""
@@ -3304,9 +3307,6 @@ msgstr ""
 msgid "Cannot refer to a group milestone by an internal id!"
 msgstr ""
-msgid "Cannot render the image. Maximum character count (%{charLimit}) has been exceeded."
-msgstr ""
-
 msgid "Cannot show preview. For previews on sketch files, they must have the file format introduced by Sketch version 43 and above."
 msgstr ""
@@ -7492,6 +7492,9 @@ msgstr ""
 msgid "Enabling this will only make licensed EE features available to projects if the project namespace's plan includes the feature or if the project is public."
 msgstr ""
+msgid "Encountered an error while rendering: %{err}"
+msgstr ""
+
 msgid "End date"
 msgstr ""
@@ -8380,7 +8383,7 @@ msgstr ""
 msgid "Failed to create a branch for this issue. Please try again."
 msgstr ""
-msgid "Failed to create repository via gitlab-shell"
+msgid "Failed to create repository"
 msgstr ""
 msgid "Failed to create resources"
@@ -15148,6 +15151,9 @@ msgstr ""
 msgid "Profiles|your account"
 msgstr ""
+msgid "Profile|%{job_title} at %{organization}"
+msgstr ""
+
 msgid "Profiling - Performance bar"
 msgstr ""
@@ -17679,9 +17685,6 @@ msgstr ""
 msgid "Select user"
 msgstr ""
-msgid "Select your role"
-msgstr ""
-
 msgid "Selected levels cannot be used by non-admin users for groups, projects or snippets. If the public level is restricted, user profiles are only visible to logged in users."
 msgstr ""
@@ -22362,6 +22365,9 @@ msgstr ""
 msgid "Warning:"
 msgstr ""
+msgid "Warning: Displaying this diagram might cause performance issues on this page."
+msgstr ""
+
 msgid "We could not determine the path to remove the epic"
 msgstr ""

package.json

@@ -96,7 +96,7 @@
     "katex": "^0.10.0",
     "lodash": "^4.17.15",
     "marked": "^0.3.12",
-    "mermaid": "^8.4.5",
+    "mermaid": "^8.4.8",
     "monaco-editor": "^0.18.1",
     "monaco-editor-webpack-plugin": "^1.7.0",
     "mousetrap": "^1.4.6",

qa/qa/page/merge_request/show.rb

@@ -14,6 +14,7 @@ module QA
           element :dropdown_toggle
           element :download_email_patches
           element :download_plain_diff
+          element :open_in_web_ide_button
         end
         view 'app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue' do
@@ -219,6 +220,10 @@ module QA
         def wait_for_loading
           finished_loading? && has_no_element?(:skeleton_note)
         end
+
+        def click_open_in_web_ide
+          click_element :open_in_web_ide_button
+        end
       end
     end
   end

qa/qa/page/project/web_ide/edit.rb

@@ -48,6 +48,10 @@ module QA
             element :start_new_mr_checkbox
           end
+          view 'app/assets/javascripts/ide/components/repo_editor.vue' do
+            element :editor_container
+          end
+
           def has_file?(file_name)
             within_element(:file_list) do
               page.has_content? file_name
@@ -113,6 +117,17 @@ module QA
             raise "The changes do not appear to have been committed successfully." unless commit_success_msg_shown
           end
+
+          def add_to_modified_content(content)
+            finished_loading?
+            modified_text_area.set content
+          end
+
+          def modified_text_area
+            within_element(:editor_container) do
+              find('.modified textarea.inputarea')
+            end
+          end
         end
       end
     end

qa/qa/specs/features/browser_ui/3_create/web_ide/review_merge_request_spec.rb

+# frozen_string_literal: true
+
+module QA
+  context 'Create', quarantine: { type: :new } do
+    describe 'Review a merge request in Web IDE' do
+      let(:new_file) { 'awesome_new_file.txt' }
+      let(:review_text) { 'Reviewed ' }
+
+      let(:merge_request) do
+        Resource::MergeRequest.fabricate_via_api! do |mr|
+          mr.file_name = new_file
+          mr.file_content = 'Text'
+        end
+      end
+
+      before do
+        Flow::Login.sign_in
+
+        merge_request.visit!
+      end
+
+      it 'opens and edits a merge request in Web IDE' do
+        Page::MergeRequest::Show.perform do |show|
+          show.click_open_in_web_ide
+        end
+
+        Page::Project::WebIDE::Edit.perform do |ide|
+          ide.has_file?(new_file)
+          ide.add_to_modified_content(review_text)
+          ide.commit_changes
+        end
+
+        merge_request.visit!
+
+        Page::MergeRequest::Show.perform do |show|
+          show.click_diffs_tab
+          expect(show).to have_content(review_text)
+        end
+      end
+    end
+  end
+end

spec/controllers/projects/tags/releases_controller_spec.rb

@@ -67,13 +67,13 @@ describe Projects::Tags::ReleasesController do
       expect(response).to have_gitlab_http_status(:found)
     end
-    it 'deletes release when description is empty' do
-      initial_releases_count = project.releases.count
-      response = update_release(release.tag, "")
-      expect(initial_releases_count).to eq(1)
-      expect(project.releases.count).to eq(0)
+    it 'does not delete release when description is empty' do
+      expect do
+        update_release(tag, "")
+      end.not_to change { project.releases.count }
+      expect(release.reload.description).to eq("")
       expect(response).to have_gitlab_http_status(:found)
     end

spec/features/markdown/mermaid_spec.rb

@@ -38,7 +38,9 @@ describe 'Mermaid rendering', :js do
     visit project_issue_path(project, issue)
-    expected = '<text><tspan xml:space="preserve" dy="1em" x="1">Line 1</tspan><tspan xml:space="preserve" dy="1em" x="1">Line 2</tspan></text>'
+    wait_for_requests
+
+    expected = '<text style=""><tspan xml:space="preserve" dy="1em" x="1">Line 1</tspan><tspan xml:space="preserve" dy="1em" x="1">Line 2</tspan></text>'
     expect(page.html.scan(expected).count).to be(4)
   end
@@ -121,4 +123,40 @@ describe 'Mermaid rendering', :js do
     expect(svg[:width].to_i).to eq(100)
     expect(svg[:height].to_i).to eq(0)
   end
+
+  it 'display button when diagram exceeds length', :js do
+    graph_edges = "A-->B;B-->A;" * 420
+
+    description = <<~MERMAID
+    ```mermaid
+    graph LR
+    #{graph_edges}
+    ```
+    MERMAID
+
+    project = create(:project, :public)
+    issue = create(:issue, project: project, description: description)
+
+    visit project_issue_path(project, issue)
+
+    page.within('.description') do
+      expect(page).not_to have_selector('svg')
+
+      expect(page).to have_selector('pre.mermaid')
+
+      expect(page).to have_selector('.lazy-alert-shown')
+
+      expect(page).to have_selector('.js-lazy-render-mermaid-container')
+    end
+
+    wait_for_requests
+
+    find('.js-lazy-render-mermaid').click
+
+    page.within('.description') do
+      expect(page).to have_selector('svg')
+
+      expect(page).not_to have_selector('.js-lazy-render-mermaid-container')
+    end
+  end
 end

spec/features/profiles/user_edit_profile_spec.rb

@@ -15,6 +15,11 @@ describe 'User edit profile' do
     wait_for_requests if respond_to?(:wait_for_requests)
   end
+  def visit_user
+    visit user_path(user)
+    wait_for_requests
+  end
+
   it 'changes user profile' do
     fill_in 'user_skype', with: 'testskype'
     fill_in 'user_linkedin', with: 'testlinkedin'
@@ -22,8 +27,8 @@ describe 'User edit profile' do
     fill_in 'user_website_url', with: 'testurl'
     fill_in 'user_location', with: 'Ukraine'
     fill_in 'user_bio', with: 'I <3 GitLab'
+    fill_in 'user_job_title', with: 'Frontend Engineer'
     fill_in 'user_organization', with: 'GitLab'
-    select 'Data Analyst', from: 'user_role'
     submit_settings
     expect(user.reload).to have_attributes(
@@ -32,8 +37,8 @@ describe 'User edit profile' do
       twitter: 'testtwitter',
       website_url: 'testurl',
       bio: 'I <3 GitLab',
-      organization: 'GitLab',
-      role: 'data_analyst'
+      job_title: 'Frontend Engineer',
+      organization: 'GitLab'
     )
     expect(find('#user_location').value).to eq 'Ukraine'
@@ -94,11 +99,6 @@ describe 'User edit profile' do
   end
   context 'user status', :js do
-    def visit_user
-      visit user_path(user)
-      wait_for_requests
-    end
-
     def select_emoji(emoji_name, is_modal = false)
       emoji_menu_class = is_modal ? '.js-modal-status-emoji-menu' : '.js-status-emoji-menu'
       toggle_button = find('.js-toggle-emoji-menu')
@@ -381,4 +381,40 @@ describe 'User edit profile' do
       end
     end
   end
+
+  context 'work information', :js do
+    context 'when job title and organziation are entered' do
+      it "shows job title and organzation on user's profile" do
+        fill_in 'user_job_title', with: 'Frontend Engineer'
+        fill_in 'user_organization', with: 'GitLab - work info test'
+        submit_settings
+
+        visit_user
+
+        expect(page).to have_content('Frontend Engineer at GitLab - work info test')
+      end
+    end
+
+    context 'when only job title is entered' do
+      it "shows only job title on user's profile" do
+        fill_in 'user_job_title', with: 'Frontend Engineer - work info test'
+        submit_settings
+
+        visit_user
+
+        expect(page).to have_content('Frontend Engineer - work info test')
+      end
+    end
+
+    context 'when only organization is entered' do
+      it "shows only organization on user's profile" do
+        fill_in 'user_organization', with: 'GitLab - work info test'
+        submit_settings
+
+        visit_user
+
+        expect(page).to have_content('GitLab - work info test')
+      end
+    end
+  end
 end

spec/features/users/show_spec.rb

@@ -26,6 +26,34 @@ describe 'User page' do
       expect(page).not_to have_content("This user has a private profile")
     end
+
+    context 'work information' do
+      subject { visit(user_path(user)) }
+
+      it 'shows job title and organization details' do
+        user.update(organization: 'GitLab - work info test', job_title: 'Frontend Engineer')
+
+        subject
+
+        expect(page).to have_content('Frontend Engineer at GitLab - work info test')
+      end
+
+      it 'shows job title' do
+        user.update(organization: nil, job_title: 'Frontend Engineer - work info test')
+
+        subject
+
+        expect(page).to have_content('Frontend Engineer - work info test')
+      end
+
+      it 'shows organization details' do
+        user.update(organization: 'GitLab - work info test', job_title: '')
+
+        subject
+
+        expect(page).to have_content('GitLab - work info test')
+      end
+    end
   end
   context 'with private profile' do

spec/frontend/vue_shared/components/user_popover/user_popover_spec.js

-import { GlSkeletonLoading } from '@gitlab/ui';
+import { GlSkeletonLoading, GlSprintf } from '@gitlab/ui';
 import { shallowMount } from '@vue/test-utils';
 import UserPopover from '~/vue_shared/components/user_popover/user_popover.vue';
 import Icon from '~/vue_shared/components/icon.vue';
@@ -11,6 +11,7 @@ const DEFAULT_PROPS = {
     location: 'Vienna',
     bio: null,
     organization: null,
+    jobTitle: null,
     status: null,
   },
 };
@@ -39,6 +40,9 @@ describe('User Popover Component', () => {
         target: findTarget(),
         ...props,
       },
+      stubs: {
+        'gl-sprintf': GlSprintf,
+      },
       ...options,
     });
   };
@@ -56,6 +60,7 @@ describe('User Popover Component', () => {
               location: null,
               bio: null,
               organization: null,
+              jobTitle: null,
               status: null,
             },
           },
@@ -85,51 +90,125 @@ describe('User Popover Component', () => {
   });
   describe('job data', () => {
-    it('should show only bio if no organization is available', () => {
-      const user = { ...DEFAULT_PROPS.user, bio: 'Engineer' };
+    const findWorkInformation = () => wrapper.find({ ref: 'workInformation' });
+    const findBio = () => wrapper.find({ ref: 'bio' });
+
+    it('should show only bio if organization and job title are not available', () => {
+      const user = { ...DEFAULT_PROPS.user, bio: 'My super interesting bio' };
       createWrapper({ user });
-      expect(wrapper.text()).toContain('Engineer');
+      expect(findBio().text()).toBe('My super interesting bio');
+      expect(findWorkInformation().exists()).toBe(false);
     });
-    it('should show only organization if no bio is available', () => {
+    it('should show only organization if job title is not available', () => {
       const user = { ...DEFAULT_PROPS.user, organization: 'GitLab' };
       createWrapper({ user });
-      expect(wrapper.text()).toContain('GitLab');
+      expect(findWorkInformation().text()).toBe('GitLab');
+    });
+
+    it('should show only job title if organization is not available', () => {
+      const user = { ...DEFAULT_PROPS.user, jobTitle: 'Frontend Engineer' };
+
+      createWrapper({ user });
+
+      expect(findWorkInformation().text()).toBe('Frontend Engineer');
+    });
+
+    it('should show organization and job title if they are both available', () => {
+      const user = {
+        ...DEFAULT_PROPS.user,
+        organization: 'GitLab',
+        jobTitle: 'Frontend Engineer',
+      };
+
+      createWrapper({ user });
+
+      expect(findWorkInformation().text()).toBe('Frontend Engineer at GitLab');
+    });
+
+    it('should display bio and job info in separate lines', () => {
+      const user = {
+        ...DEFAULT_PROPS.user,
+        bio: 'My super interesting bio',
+        organization: 'GitLab',
+      };
+
+      createWrapper({ user });
+
+      expect(findBio().text()).toBe('My super interesting bio');
+      expect(findWorkInformation().text()).toBe('GitLab');
     });
-    it('should display bio and organization in separate lines', () => {
-      const user = { ...DEFAULT_PROPS.user, bio: 'Engineer', organization: 'GitLab' };
+    it('should not encode special characters in bio', () => {
+      const user = {
+        ...DEFAULT_PROPS.user,
+        bio: 'I like <html> & CSS',
+      };
       createWrapper({ user });
-      expect(wrapper.find('.js-bio').text()).toContain('Engineer');
-      expect(wrapper.find('.js-organization').text()).toContain('GitLab');
+      expect(findBio().text()).toBe('I like <html> & CSS');
     });
-    it('should not encode special characters in bio and organization', () => {
+    it('should not encode special characters in organization', () => {
       const user = {
         ...DEFAULT_PROPS.user,
-        bio: 'Manager & Team Lead',
         organization: 'Me & my <funky> Company',
       };
       createWrapper({ user });
-      expect(wrapper.find('.js-bio').text()).toContain('Manager & Team Lead');
-      expect(wrapper.find('.js-organization').text()).toContain('Me & my <funky> Company');
+      expect(findWorkInformation().text()).toBe('Me & my <funky> Company');
+    });
+
+    it('should not encode special characters in job title', () => {
+      const user = {
+        ...DEFAULT_PROPS.user,
+        jobTitle: 'Manager & Team Lead',
+      };
+
+      createWrapper({ user });
+
+      expect(findWorkInformation().text()).toBe('Manager & Team Lead');
+    });
+
+    it('should not encode special characters when both job title and organization are set', () => {
+      const user = {
+        ...DEFAULT_PROPS.user,
+        jobTitle: 'Manager & Team Lead',
+        organization: 'Me & my <funky> Company',
+      };
+
+      createWrapper({ user });
+
+      expect(findWorkInformation().text()).toBe('Manager & Team Lead at Me & my <funky> Company');
     });
     it('shows icon for bio', () => {
+      const user = {
+        ...DEFAULT_PROPS.user,
+        bio: 'My super interesting bio',
+      };
+
+      createWrapper({ user });
+
       expect(wrapper.findAll(Icon).filter(icon => icon.props('name') === 'profile').length).toEqual(
         1,
       );
     });
     it('shows icon for organization', () => {
+      const user = {
+        ...DEFAULT_PROPS.user,
+        organization: 'GitLab',
+      };
+
+      createWrapper({ user });
+
       expect(wrapper.findAll(Icon).filter(icon => icon.props('name') === 'work').length).toEqual(1);
     });
   });

spec/helpers/users_helper_spec.rb

@@ -178,4 +178,42 @@ describe UsersHelper do
       end
     end
   end
+
+  describe '#work_information' do
+    subject { helper.work_information(user) }
+
+    context 'when both job_title and organization are present' do
+      let(:user) { build(:user, organization: 'GitLab', job_title: 'Frontend Engineer') }
+
+      it 'returns job title concatenated with organization' do
+        is_expected.to eq('Frontend Engineer at GitLab')
+      end
+    end
+
+    context 'when only organization is present' do
+      let(:user) { build(:user, organization: 'GitLab') }
+
+      it "returns organization" do
+        is_expected.to eq('GitLab')
+      end
+    end
+
+    context 'when only job_title is present' do
+      let(:user) { build(:user, job_title: 'Frontend Engineer') }
+
+      it 'returns job title' do
+        is_expected.to eq('Frontend Engineer')
+      end
+    end
+
+    context 'when neither organization nor job_title are present' do
+      it { is_expected.to be_nil }
+    end
+
+    context 'when user parameter is nil' do
+      let(:user) { nil }
+
+      it { is_expected.to be_nil }
+    end
+  end
 end

spec/lib/backup/repository_spec.rb

@@ -50,9 +50,9 @@ describe Backup::Repository do
     describe 'command failure' do
       before do
-        allow_next_instance_of(Gitlab::Shell) do |instance|
-          allow(instance).to receive(:create_repository).and_return(false)
-        end
+        # Allow us to set expectations on the project directly
+        expect(Project).to receive(:find_each).and_yield(project)
+        expect(project.repository).to receive(:create_repository) { raise 'Fail in tests' }
       end
       context 'hashed storage' do

spec/lib/gitlab/legacy_github_import/importer_spec.rb

@@ -173,10 +173,6 @@ describe Gitlab::LegacyGithubImport::Importer do
         ]
       }
-      unless project.gitea_import?
-        error[:errors] << { type: :release, url: "#{api_root}/repos/octocat/Hello-World/releases/2", errors: "Validation failed: Description can't be blank" }
-      end
-
       described_class.new(project).execute
       expect(project.import_state.last_error).to eq error.to_json

spec/lib/gitlab/shell_spec.rb

@@ -7,18 +7,17 @@ describe Gitlab::Shell do
   let_it_be(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
   let(:gitlab_shell) { described_class.new }
-  let(:popen_vars) { { 'GIT_TERMINAL_PROMPT' => ENV['GIT_TERMINAL_PROMPT'] } }
-  let(:timeout) { Gitlab.config.gitlab_shell.git_timeout }
-  before do
-    allow(Project).to receive(:find).and_return(project)
-  end
-
-  it { is_expected.to respond_to :create_repository }
   it { is_expected.to respond_to :remove_repository }
   it { is_expected.to respond_to :fork_repository }
-  it { expect(gitlab_shell.url_to_repo('diaspora')).to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + "diaspora.git") }
+  describe '.url_to_repo' do
+    let(:full_path) { 'diaspora/disaspora-rails' }
+
+    subject { described_class.url_to_repo(full_path) }
+
+    it { is_expected.to eq(Gitlab.config.gitlab_shell.ssh_path_prefix + full_path + '.git') }
+  end
   describe 'memoized secret_token' do
     let(:secret_file) { 'tmp/tests/.secret_shell_test' }
@@ -49,37 +48,12 @@ describe Gitlab::Shell do
   describe 'projects commands' do
     let(:gitlab_shell_path) { File.expand_path('tmp/tests/gitlab-shell') }
     let(:projects_path) { File.join(gitlab_shell_path, 'bin/gitlab-projects') }
-    let(:gitlab_shell_hooks_path) { File.join(gitlab_shell_path, 'hooks') }
     before do
       allow(Gitlab.config.gitlab_shell).to receive(:path).and_return(gitlab_shell_path)
       allow(Gitlab.config.gitlab_shell).to receive(:git_timeout).and_return(800)
     end
-    describe '#create_repository' do
-      let(:repository_storage) { 'default' }
-      let(:repository_storage_path) do
-        Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-          Gitlab.config.repositories.storages[repository_storage].legacy_disk_path
-        end
-      end
-      let(:repo_name) { 'project/path' }
-      let(:created_path) { File.join(repository_storage_path, repo_name + '.git') }
-
-      after do
-        FileUtils.rm_rf(created_path)
-      end
-
-      it 'returns false when the command fails' do
-        FileUtils.mkdir_p(File.dirname(created_path))
-        # This file will block the creation of the repo's .git directory. That
-        # should cause #create_repository to fail.
-        FileUtils.touch(created_path)
-
-        expect(gitlab_shell.create_repository(repository_storage, repo_name, repo_name)).to be_falsy
-      end
-    end
-
     describe '#remove_repository' do
       let!(:project) { create(:project, :repository, :legacy_storage) }
       let(:disk_path) { "#{project.disk_path}.git" }

spec/models/concerns/bulk_insert_safe_spec.rb

@@ -22,6 +22,18 @@ describe BulkInsertSafe do
       algorithm: 'aes-256-gcm',
       key: Settings.attr_encrypted_db_key_base_32,
       insecure_mode: false
+
+    default_value_for :enum_value, 'case_1'
+    default_value_for :secret_value, 'my-secret'
+    default_value_for :sha_value, '2fd4e1c67a2d28fced849ee1bb76e7391b93eb12'
+
+    def self.valid_list(count)
+      Array.new(count) { |n| new(name: "item-#{n}") }
+    end
+
+    def self.invalid_list(count)
+      Array.new(count) { new }
+    end
   end
   module InheritedUnsafeMethods
@@ -48,6 +60,8 @@ describe BulkInsertSafe do
         t.text :encrypted_secret_value, null: false
         t.string :encrypted_secret_value_iv, null: false
         t.binary :sha_value, null: false, limit: 20
+
+        t.index :name, unique: true
       end
     end
@@ -60,87 +74,95 @@ describe BulkInsertSafe do
     end
   end
-  def build_valid_items_for_bulk_insertion
-    Array.new(10) do |n|
-      BulkInsertItem.new(
-        name: "item-#{n}",
-        enum_value: 'case_1',
-        secret_value: 'my-secret',
-        sha_value: '2fd4e1c67a2d28fced849ee1bb76e7391b93eb12'
-      )
+  describe BulkInsertItem do
+    it_behaves_like 'a BulkInsertSafe model', described_class do
+      let(:valid_items_for_bulk_insertion) { described_class.valid_list(10) }
+      let(:invalid_items_for_bulk_insertion) { described_class.invalid_list(10) }
     end
-  end
-  def build_invalid_items_for_bulk_insertion
-    Array.new(10) do
-      BulkInsertItem.new(
-        name: nil, # requires `name` to be set
-        enum_value: 'case_1',
-        secret_value: 'my-secret',
-        sha_value: '2fd4e1c67a2d28fced849ee1bb76e7391b93eb12'
-      )
+    context 'when inheriting class methods' do
+      it 'raises an error when method is not bulk-insert safe' do
+        expect { described_class.include(InheritedUnsafeMethods) }
+          .to raise_error(described_class::MethodNotAllowedError)
+      end
+
+      it 'does not raise an error when method is bulk-insert safe' do
+        expect { described_class.include(InheritedSafeMethods) }.not_to raise_error
+      end
     end
-  end
-  it_behaves_like 'a BulkInsertSafe model', BulkInsertItem do
-    let(:valid_items_for_bulk_insertion) { build_valid_items_for_bulk_insertion }
-    let(:invalid_items_for_bulk_insertion) { build_invalid_items_for_bulk_insertion }
-  end
-  context 'when inheriting class methods' do
-    it 'raises an error when method is not bulk-insert safe' do
-      expect { BulkInsertItem.include(InheritedUnsafeMethods) }.to(
-        raise_error(subject::MethodNotAllowedError))
+    context 'primary keys' do
+      it 'raises error if primary keys are set prior to insertion' do
+        item = described_class.new(name: 'valid', id: 10)
+        expect { described_class.bulk_insert!([item]) }
+          .to raise_error(described_class::PrimaryKeySetError)
+      end
     end
-    it 'does not raise an error when method is bulk-insert safe' do
-      expect { BulkInsertItem.include(InheritedSafeMethods) }.not_to raise_error
-    end
-  end
-  context 'primary keys' do
-    it 'raises error if primary keys are set prior to insertion' do
-      items = build_valid_items_for_bulk_insertion
-      items.each_with_index do |item, n|
-        item.id = n
+    describe '.bulk_insert!' do
+      it 'inserts items in the given number of batches' do
+        items = described_class.valid_list(10)
+
+        expect(ActiveRecord::InsertAll).to receive(:new).twice.and_call_original
+        described_class.bulk_insert!(items, batch_size: 5)
       end
-      expect { BulkInsertItem.bulk_insert!(items) }.to raise_error(subject::PrimaryKeySetError)
-    end
-  end
-  describe '.bulk_insert!' do
-    it 'inserts items in the given number of batches' do
-      items = build_valid_items_for_bulk_insertion
-      expect(items.size).to eq(10)
-      expect(BulkInsertItem).to receive(:insert_all!).twice
-      BulkInsertItem.bulk_insert!(items, batch_size: 5)
-    end
-    it 'items can be properly fetched from database' do
-      items = build_valid_items_for_bulk_insertion
-      BulkInsertItem.bulk_insert!(items)
-      attribute_names = BulkInsertItem.attribute_names - %w[id]
-      expect(BulkInsertItem.last(items.size).pluck(*attribute_names)).to eq(
-        items.pluck(*attribute_names))
+      it 'items can be properly fetched from database' do
+        items = described_class.valid_list(10)
+        described_class.bulk_insert!(items)
+        attribute_names = described_class.attribute_names - %w[id]
+        expect(described_class.last(items.size).pluck(*attribute_names)).to eq(
+          items.pluck(*attribute_names))
+      end
+      it 'rolls back the transaction when any item is invalid' do
+        # second batch is bad
+        all_items = described_class.valid_list(10) +
+          described_class.invalid_list(10)
+        expect do
+          described_class.bulk_insert!(all_items, batch_size: 2) rescue nil
+        end.not_to change { described_class.count }
+      end
+      it 'does nothing and returns true when items are empty' do
+        expect(described_class.bulk_insert!([])).to be(true)
+        expect(described_class.count).to eq(0)
+      end
     end
-    it 'rolls back the transaction when any item is invalid' do
-      # second batch is bad
-      all_items = build_valid_items_for_bulk_insertion + build_invalid_items_for_bulk_insertion
-      batch_size = all_items.size / 2
-      expect do
-        BulkInsertItem.bulk_insert!(all_items, batch_size: batch_size) rescue nil
-      end.not_to change { BulkInsertItem.count }
-    end
-    it 'does nothing and returns true when items are empty' do
-      expect(BulkInsertItem.bulk_insert!([])).to be(true)
-      expect(BulkInsertItem.count).to eq(0)
+    context 'when duplicate items are to be inserted' do
+      let!(:existing_object) { described_class.create!(name: 'duplicate', secret_value: 'old value') }
+      let(:new_object) { described_class.new(name: 'duplicate', secret_value: 'new value') }
+
+      describe '.bulk_insert!' do
+        context 'when skip_duplicates is set to false' do
+          it 'raises an exception' do
+            expect { described_class.bulk_insert!([new_object], skip_duplicates: false) }
+              .to raise_error(ActiveRecord::RecordNotUnique)
+          end
+        end
+
+        context 'when skip_duplicates is set to true' do
+          it 'does not update existing object' do
+            described_class.bulk_insert!([new_object], skip_duplicates: true)
+
+            expect(existing_object.reload.secret_value).to eq('old value')
+          end
+        end
+      end
+      describe '.bulk_upsert!' do
+        it 'updates existing object' do
+          described_class.bulk_upsert!([new_object], unique_by: %w[name])
+          expect(existing_object.reload.secret_value).to eq('new value')
+        end
+      end
     end
   end
 end

spec/models/project_spec.rb

@@ -1921,30 +1921,15 @@ describe Project do
   describe '#create_repository' do
     let(:project) { create(:project, :repository) }
-    let(:shell) { Gitlab::Shell.new }
-
-    before do
-      allow(project).to receive(:gitlab_shell).and_return(shell)
-    end
     context 'using a regular repository' do
       it 'creates the repository' do
-        expect(shell).to receive(:create_repository)
-          .with(project.repository_storage, project.disk_path, project.full_path)
-          .and_return(true)
-
-        expect(project.repository).to receive(:after_create)
-
+        expect(project.repository).to receive(:create_repository)
         expect(project.create_repository).to eq(true)
       end
       it 'adds an error if the repository could not be created' do
-        expect(shell).to receive(:create_repository)
-          .with(project.repository_storage, project.disk_path, project.full_path)
-          .and_return(false)
-
-        expect(project.repository).not_to receive(:after_create)
-
+        expect(project.repository).to receive(:create_repository) { raise 'Fail in test' }
         expect(project.create_repository).to eq(false)
         expect(project.errors).not_to be_empty
       end
@@ -1953,7 +1938,7 @@ describe Project do
     context 'using a forked repository' do
       it 'does nothing' do
         expect(project).to receive(:forked?).and_return(true)
-        expect(shell).not_to receive(:create_repository)
+        expect(project.repository).not_to receive(:create_repository)
         project.create_repository
       end
@@ -1962,28 +1947,16 @@ describe Project do
   describe '#ensure_repository' do
     let(:project) { create(:project, :repository) }
-    let(:shell) { Gitlab::Shell.new }
-
-    before do
-      allow(project).to receive(:gitlab_shell).and_return(shell)
-    end
     it 'creates the repository if it not exist' do
-      allow(project).to receive(:repository_exists?)
-        .and_return(false)
-
-      allow(shell).to receive(:create_repository)
-        .with(project.repository_storage, project.disk_path, project.full_path)
-        .and_return(true)
-
+      allow(project).to receive(:repository_exists?).and_return(false)
       expect(project).to receive(:create_repository).with(force: true)
       project.ensure_repository
     end
     it 'does not create the repository if it exists' do
-      allow(project).to receive(:repository_exists?)
-        .and_return(true)
+      allow(project).to receive(:repository_exists?).and_return(true)
       expect(project).not_to receive(:create_repository)
@@ -1992,13 +1965,8 @@ describe Project do
     it 'creates the repository if it is a fork' do
       expect(project).to receive(:forked?).and_return(true)
-
-      allow(project).to receive(:repository_exists?)
-        .and_return(false)
-
-      expect(shell).to receive(:create_repository)
-        .with(project.repository_storage, project.disk_path, project.full_path)
-        .and_return(true)
+      expect(project).to receive(:repository_exists?).and_return(false)
+      expect(project.repository).to receive(:create_repository) { true }
       project.ensure_repository
     end

spec/models/project_wiki_spec.rb

@@ -34,7 +34,7 @@ describe ProjectWiki do
   describe "#url_to_repo" do
     it "returns the correct ssh url to the repo" do
-      expect(subject.url_to_repo).to eq(gitlab_shell.url_to_repo(subject.full_path))
+      expect(subject.url_to_repo).to eq(Gitlab::Shell.url_to_repo(subject.full_path))
     end
   end
@@ -97,9 +97,7 @@ describe ProjectWiki do
     it "raises CouldNotCreateWikiError if it can't create the wiki repository" do
       # Create a fresh project which will not have a wiki
       project_wiki = described_class.new(create(:project), user)
-      gitlab_shell = double(:gitlab_shell)
-      allow(gitlab_shell).to receive(:create_wiki_repository)
-      allow(project_wiki).to receive(:gitlab_shell).and_return(gitlab_shell)
+      expect(project_wiki.repository).to receive(:create_if_not_exists) { false }
       expect { project_wiki.send(:wiki) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError)
     end
@@ -416,26 +414,12 @@ describe ProjectWiki do
     end
   end
-  describe '#create_repo!' do
-    let(:project) { create(:project) }
-
-    it 'creates a repository' do
-      expect(raw_repository.exists?).to eq(false)
-      expect(subject.repository).to receive(:after_create)
-
-      subject.send(:create_repo!, raw_repository)
-
-      expect(raw_repository.exists?).to eq(true)
-    end
-  end
-
   describe '#ensure_repository' do
     let(:project) { create(:project) }
     it 'creates the repository if it not exist' do
       expect(raw_repository.exists?).to eq(false)
-      expect(subject).to receive(:create_repo!).and_call_original
       subject.ensure_repository
       expect(raw_repository.exists?).to eq(true)

spec/models/release_spec.rb

@@ -20,7 +20,6 @@ RSpec.describe Release do
   describe 'validation' do
     it { is_expected.to validate_presence_of(:project) }
-    it { is_expected.to validate_presence_of(:description) }
     it { is_expected.to validate_presence_of(:tag) }
     context 'when a release exists in the database without a name' do

spec/requests/api/internal/base_spec.rb

@@ -262,6 +262,8 @@ describe API::Internal::Base do
   describe "POST /internal/allowed", :clean_gitlab_redis_shared_state do
     context "access granted" do
+      let(:env) { {} }
+
       around do |example|
         Timecop.freeze { example.run }
       end
@@ -270,30 +272,32 @@ describe API::Internal::Base do
         project.add_developer(user)
       end
-      context 'with env passed as a JSON' do
-        let(:gl_repository) { Gitlab::GlRepository::WIKI.identifier_for_container(project) }
-
-        it 'sets env in RequestStore' do
-          obj_dir_relative = './objects'
-          alt_obj_dirs_relative = ['./alt-objects-1', './alt-objects-2']
-          expect(Gitlab::Git::HookEnv).to receive(:set).with(gl_repository, {
-            'GIT_OBJECT_DIRECTORY_RELATIVE' => obj_dir_relative,
-            'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => alt_obj_dirs_relative
-          })
-          push(key, project.wiki, env: {
-            GIT_OBJECT_DIRECTORY_RELATIVE: obj_dir_relative,
-            GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE: alt_obj_dirs_relative
-          }.to_json)
-          expect(response).to have_gitlab_http_status(:ok)
+      shared_examples 'sets hook env' do
+        context 'with env passed as a JSON' do
+          let(:obj_dir_relative) { './objects' }
+          let(:alt_obj_dirs_relative) { ['./alt-objects-1', './alt-objects-2'] }
+          let(:env) do
+            {
+              GIT_OBJECT_DIRECTORY_RELATIVE: obj_dir_relative,
+              GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE: alt_obj_dirs_relative
+            }
+          end
+          it 'sets env in RequestStore' do
+            expect(Gitlab::Git::HookEnv).to receive(:set).with(gl_repository, env.stringify_keys)
+            subject
+            expect(response).to have_gitlab_http_status(:ok)
+          end
         end
       end
       context "git push with project.wiki" do
+        subject { push(key, project.wiki, env: env.to_json) }
+
         it 'responds with success' do
-          push(key, project.wiki)
+          subject
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response["status"]).to be_truthy
@@ -301,6 +305,10 @@ describe API::Internal::Base do
           expect(json_response["gl_repository"]).to eq("wiki-#{project.id}")
           expect(user.reload.last_activity_on).to be_nil
         end
+
+        it_behaves_like 'sets hook env' do
+          let(:gl_repository) { Gitlab::GlRepository::WIKI.identifier_for_container(project) }
+        end
       end
       context "git pull with project.wiki" do
@@ -328,8 +336,10 @@ describe API::Internal::Base do
       end
       context 'git push with personal snippet' do
+        subject { push(key, personal_snippet, env: env.to_json) }
+
         it 'responds with success' do
-          push(key, personal_snippet)
+          subject
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response["status"]).to be_truthy
@@ -338,8 +348,9 @@ describe API::Internal::Base do
           expect(user.reload.last_activity_on).to be_nil
         end
-        it_behaves_like 'snippets with disabled feature flag' do
-          subject { push(key, personal_snippet) }
+        it_behaves_like 'snippets with disabled feature flag'
+        it_behaves_like 'sets hook env' do
+          let(:gl_repository) { Gitlab::GlRepository::SNIPPET.identifier_for_container(personal_snippet) }
         end
       end
@@ -360,8 +371,10 @@ describe API::Internal::Base do
       end
       context 'git push with project snippet' do
+        subject { push(key, project_snippet, env: env.to_json) }
+
         it 'responds with success' do
-          push(key, project_snippet)
+          subject
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response["status"]).to be_truthy
@@ -370,8 +383,9 @@ describe API::Internal::Base do
           expect(user.reload.last_activity_on).to be_nil
         end
-        it_behaves_like 'snippets with disabled feature flag' do
-          subject { push(key, project_snippet) }
+        it_behaves_like 'snippets with disabled feature flag'
+        it_behaves_like 'sets hook env' do
+          let(:gl_repository) { Gitlab::GlRepository::SNIPPET.identifier_for_container(project_snippet) }
         end
       end

spec/requests/api/releases_spec.rb

@@ -406,6 +406,22 @@ describe API::Releases do
       expect(project.releases.last.description).to eq('Super nice release')
     end
+    it 'creates a new release without description' do
+      params = {
+          name: 'New release without description',
+          tag_name: 'v0.1',
+          released_at: '2019-03-25 10:00:00'
+      }
+
+      expect do
+        post api("/projects/#{project.id}/releases", maintainer), params: params
+      end.to change { Release.count }.by(1)
+
+      expect(project.releases.last.name).to eq('New release without description')
+      expect(project.releases.last.tag).to eq('v0.1')
+      expect(project.releases.last.description).to eq(nil)
+    end
+
     it 'sets the released_at to the current time if the released_at parameter is not provided' do
       now = Time.zone.parse('2015-08-25 06:00:00Z')
       Timecop.freeze(now) do
@@ -451,26 +467,6 @@ describe API::Releases do
       expect(project.releases.last.released_at).to eq('2019-03-25T01:00:00Z')
     end
-    context 'when description is empty' do
-      let(:params) do
-        {
-          name: 'New release',
-          tag_name: 'v0.1',
-          description: ''
-        }
-      end
-
-      it 'returns an error as validation failure' do
-        expect do
-          post api("/projects/#{project.id}/releases", maintainer), params: params
-        end.not_to change { Release.count }
-
-        expect(response).to have_gitlab_http_status(:bad_request)
-        expect(json_response['message'])
-          .to eq("Validation failed: Description can't be blank")
-      end
-    end
-
     it 'matches response schema' do
       post api("/projects/#{project.id}/releases", maintainer), params: params