Automatic merge of gitlab-org/gitlab master

app/assets/javascripts/groups/init_transfer_group_form.js

@@ -33,7 +33,7 @@ export default () => {
   return new Vue({
     el,
     provide: {
-      confirmDangerMessage: sprintf(i18n.confirmationMessage, { groupName }),
+      confirmDangerMessage: sprintf(i18n.confirmationMessage, { group_name: groupName }),
     },
     render(createElement) {
       return createElement(TransferGroupForm, {

app/helpers/users_helper.rb

@@ -172,6 +172,13 @@ module UsersHelper
     }
   end
 
+  def display_public_email?(user)
+    return false if user.public_email.blank?
+    return true unless user.provisioned_by_group
+
+    !Feature.enabled?(:hide_public_email_on_profile, user.provisioned_by_group)
+  end
+
   private
 
   def admin_users_paths

app/models/merge_request.rb

@@ -561,9 +561,7 @@ class MergeRequest < ApplicationRecord
     end
   end
 
-  # WIP is deprecated in favor of Draft. Currently both options are supported
-  # https://gitlab.com/gitlab-org/gitlab/-/issues/227426
-  DRAFT_REGEX = /\A*#{Regexp.union(Gitlab::Regex.merge_request_wip, Gitlab::Regex.merge_request_draft)}+\s*/i.freeze
+  DRAFT_REGEX = /\A*#{Gitlab::Regex.merge_request_draft}+\s*/i.freeze
 
   def self.work_in_progress?(title)
     !!(title =~ DRAFT_REGEX)

app/views/users/show.html.haml

@@ -112,7 +112,7 @@
                 - if Feature.enabled?(:security_auto_fix) && @user.bot?
                   = sprite_icon('question', css_class: 'gl-text-blue-600')
                 = link_to @user.short_website_url, @user.full_website_url, target: '_blank', rel: 'me noopener noreferrer nofollow', itemprop: 'url'
-            - unless @user.public_email.blank?
+            - if display_public_email?(@user)
               = render 'middle_dot_divider', stacking: true do
                 = link_to @user.public_email, "mailto:#{@user.public_email}", itemprop: 'email'
           .gl-text-gray-900

config/feature_flags/development/hide_public_email_on_profile.yml

+---
+name: hide_public_email_on_profile
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79717
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351731
+milestone: '14.8'
+type: development
+group: group::optimize
+default_enabled: false

ee/spec/features/profiles/user_visits_public_profile_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'User visits public profile' do
+  context 'when user profile is provisioned by group' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:scim_identity) { create(:scim_identity, group: group) }
+    let_it_be(:user) { create(:user, :public_email, provisioned_by_group_id: scim_identity.group.id) }
+
+    it 'hide public_email' do
+      visit(user_path(user))
+      expect(page).not_to have_content user.public_email
+    end
+
+    context 'when hide_public_email_on_profile feature flag is disabled' do
+      before do
+        stub_feature_flags(hide_public_email_on_profile: false)
+      end
+
+      it 'displays public_email' do
+        visit(user_path(user))
+        expect(page).to have_content user.public_email
+      end
+    end
+  end
+end

ee/spec/helpers/users_helper_spec.rb

@@ -104,4 +104,43 @@ RSpec.describe UsersHelper do
       end
     end
   end
+
+  describe '#display_public_email?' do
+    let_it_be(:group) { create(:group) }
+    let_it_be(:scim_identity) { create(:scim_identity, group: group) }
+
+    let(:user) { create(:user, :public_email, provisioned_by_group: scim_identity.group) }
+
+    subject { helper.display_public_email?(user) }
+
+    before do
+      stub_feature_flags hide_public_email_on_profile: false
+    end
+
+    it { is_expected.to be true }
+
+    context 'when public_email is blank' do
+      before do
+        user.update!(public_email: '')
+      end
+
+      it { is_expected.to be false }
+    end
+
+    context 'when provisioned_by_group is nil' do
+      before do
+        user.update!(provisioned_by_group: nil)
+      end
+
+      it { is_expected.to be true }
+    end
+
+    context 'when hide_public_email_on_profile is true' do
+      before do
+        stub_feature_flags hide_public_email_on_profile: true
+      end
+
+      it { is_expected.to be false }
+    end
+  end
 end

lib/gitlab/github_import/importer/releases_importer.rb

@@ -21,10 +21,12 @@ module Gitlab
         end
 
         def already_imported?(release)
-          existing_tags.include?(release.tag_name)
+          existing_tags.include?(release.tag_name) || release.tag_name.nil?
         end
 
         def build(release)
+          existing_tags.add(release.tag_name)
+
           {
             name: release.name,
             tag: release.tag_name,

lib/gitlab/legacy_github_import/release_formatter.rb

@@ -25,7 +25,7 @@ module Gitlab
       end
 
       def valid?
-        !raw_data.draft
+        !raw_data.draft && raw_data.tag_name.present?
       end
     end
   end

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -443,7 +443,7 @@ RSpec.describe Projects::MergeRequestsController do
 
     context 'when the merge request is not mergeable' do
       before do
-        merge_request.update!(title: "WIP: #{merge_request.title}")
+        merge_request.update!(title: "Draft: #{merge_request.title}")
 
         post :merge, params: base_params
       end

spec/features/merge_request/user_resolves_wip_mr_spec.rb

@@ -2,13 +2,13 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Merge request > User resolves Work in Progress', :js do
+RSpec.describe 'Merge request > User resolves Draft', :js do
   let(:project) { create(:project, :public, :repository) }
   let(:user) { project.creator }
   let(:merge_request) do
     create(:merge_request_with_diffs, source_project: project,
                                       author: user,
-                                      title: 'WIP: Bug NS-04',
+                                      title: 'Draft: Bug NS-04',
                                       merge_params: { force_remove_source_branch: '1' })
   end
 

spec/lib/gitlab/github_import/importer/releases_importer_spec.rb

@@ -52,6 +52,12 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do
 
       expect { importer.execute }.to change { Release.count }.by(1)
     end
+
+    it 'is idempotent' do
+      allow(importer).to receive(:each_release).and_return([github_release])
+      expect { importer.execute }.to change { Release.count }.by(1)
+      expect { importer.execute }.to change { Release.count }.by(0) # Idempotency check
+    end
   end
 
   describe '#build_releases' do
@@ -79,6 +85,24 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do
 
       expect(release[:description]).to eq('Release for tag 1.0')
     end
+
+    it 'does not create releases that have a NULL tag' do
+      null_tag_release = double(
+        name: 'NULL Test',
+        tag_name: nil
+      )
+
+      expect(importer).to receive(:each_release).and_return([null_tag_release])
+      expect(importer.build_releases).to be_empty
+    end
+
+    it 'does not create duplicate release tags' do
+      expect(importer).to receive(:each_release).and_return([github_release, github_release])
+
+      releases = importer.build_releases
+      expect(releases.length).to eq(1)
+      expect(releases[0][:description]).to eq('This is my release')
+    end
   end
 
   describe '#build' do

spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb

@@ -63,5 +63,13 @@ RSpec.describe Gitlab::LegacyGithubImport::ReleaseFormatter do
         expect(release.valid?).to eq false
       end
     end
+
+    context 'when release has NULL tag' do
+      let(:raw_data) { double(base_data.merge(tag_name: '')) }
+
+      it 'returns false' do
+        expect(release.valid?).to eq false
+      end
+    end
   end
 end

spec/models/merge_request_spec.rb

@@ -1369,17 +1369,17 @@ RSpec.describe MergeRequest, factory_default: :keep do
     subject { build_stubbed(:merge_request) }
 
     [
-      'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP: [WIP] WIP:',
       'draft:', 'Draft: ', '[Draft]', '[DRAFT] '
-    ].each do |wip_prefix|
-      it "detects the '#{wip_prefix}' prefix" do
-        subject.title = "#{wip_prefix}#{subject.title}"
+    ].each do |draft_prefix|
+      it "detects the '#{draft_prefix}' prefix" do
+        subject.title = "#{draft_prefix}#{subject.title}"
 
         expect(subject.work_in_progress?).to eq true
       end
     end
 
     [
+      'WIP:', 'WIP: ', '[WIP]', '[WIP] ', ' [WIP] WIP: [WIP] WIP:',
       "WIP ", "(WIP)",
       "draft", "Draft", "Draft -", "draft - ", "Draft ", "draft "
     ].each do |draft_prefix|
@@ -1390,10 +1390,10 @@ RSpec.describe MergeRequest, factory_default: :keep do
       end
     end
 
-    it "detects merge request title just saying 'wip'" do
+    it "doesn't detect merge request title just saying 'wip'" do
       subject.title = "wip"
 
-      expect(subject.work_in_progress?).to eq true
+      expect(subject.work_in_progress?).to eq false
     end
 
     it "does not detect merge request title just saying 'draft'" do
@@ -1459,29 +1459,30 @@ RSpec.describe MergeRequest, factory_default: :keep do
   describe "#wipless_title" do
     subject { build_stubbed(:merge_request) }
 
-    [
-      'WIP:', 'WIP: ', '[WIP]', '[WIP] ', '[WIP] WIP: [WIP] WIP:',
-      'draft:', 'Draft: ', '[Draft]', '[DRAFT] '
-    ].each do |wip_prefix|
-      it "removes the '#{wip_prefix}' prefix" do
+    ['draft:', 'Draft: ', '[Draft]', '[DRAFT] '].each do |draft_prefix|
+      it "removes a '#{draft_prefix}' prefix" do
         wipless_title = subject.title
-        subject.title = "#{wip_prefix}#{subject.title}"
+        subject.title = "#{draft_prefix}#{subject.title}"
 
         expect(subject.wipless_title).to eq wipless_title
       end
 
       it "is satisfies the #work_in_progress? method" do
-        subject.title = "#{wip_prefix}#{subject.title}"
+        subject.title = "#{draft_prefix}#{subject.title}"
         subject.title = subject.wipless_title
 
         expect(subject.work_in_progress?).to eq false
       end
     end
 
-    it 'removes only WIP prefix from the MR title' do
-      subject.title = 'WIP: Implement feature called WIP'
+    [
+      'WIP:', 'WIP: ', '[WIP]', '[WIP] ', '[WIP] WIP: [WIP] WIP:'
+    ].each do |wip_prefix|
+      it "doesn't remove a '#{wip_prefix}' prefix" do
+        subject.title = "#{wip_prefix}#{subject.title}"
 
-      expect(subject.wipless_title).to eq 'Implement feature called WIP'
+        expect(subject.wipless_title).to eq subject.title
+      end
     end
 
     it 'removes only draft prefix from the MR title' do

spec/services/merge_requests/create_service_spec.rb

@@ -61,19 +61,19 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
         expect(merge_request.reload).to be_preparing
       end
 
-      describe 'when marked with /wip' do
+      describe 'when marked with /draft' do
         context 'in title and in description' do
           let(:opts) do
             {
-              title: 'WIP: Awesome merge_request',
-              description: "well this is not done yet\n/wip",
+              title: 'Draft: Awesome merge_request',
+              description: "well this is not done yet\n/draft",
               source_branch: 'feature',
               target_branch: 'master',
               assignees: [user2]
             }
           end
 
-          it 'sets MR to WIP' do
+          it 'sets MR to draft' do
             expect(merge_request.work_in_progress?).to be(true)
           end
         end
@@ -89,7 +89,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
             }
           end
 
-          it 'sets MR to WIP' do
+          it 'sets MR to draft' do
             expect(merge_request.work_in_progress?).to be(true)
           end
         end

spec/services/merge_requests/update_service_spec.rb

@@ -102,16 +102,16 @@ RSpec.describe MergeRequests::UpdateService, :mailer do
           MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request2)
         end
 
-        it 'tracks Draft/WIP marking' do
+        it 'tracks Draft marking' do
           expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
             .to receive(:track_marked_as_draft_action).once.with(user: user)
 
-          opts[:title] = "WIP: #{opts[:title]}"
+          opts[:title] = "Draft: #{opts[:title]}"
 
           MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request2)
         end
 
-        it 'tracks Draft/WIP un-marking' do
+        it 'tracks Draft un-marking' do
           expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter)
             .to receive(:track_unmarked_as_draft_action).once.with(user: user)
 

spec/services/notes/create_service_spec.rb

@@ -325,11 +325,11 @@ RSpec.describe Notes::CreateService do
                   expect(issuable.work_in_progress?).to eq(can_use_quick_action)
                 }
               ),
-              # Remove WIP status
+              # Remove draft status
               QuickAction.new(
                 action_text: "/draft",
                 before_action: -> {
-                  issuable.reload.update!(title: "WIP: title")
+                  issuable.reload.update!(title: "Draft: title")
                 },
                 expectation: ->(noteable, can_use_quick_action) {
                   expect(noteable.work_in_progress?).not_to eq(can_use_quick_action)