Add latest changes from gitlab-org/gitlab@master

app/controllers/application_controller.rb

@@ -35,8 +35,9 @@ class ApplicationController < ActionController::Base
   before_action :check_impersonation_availability
   before_action :required_signup_info
 
+  prepend_around_action :set_current_context
+
   around_action :sessionless_bypass_admin_mode!, if: :sessionless_user?
-  around_action :set_current_context
   around_action :set_locale
   around_action :set_session_storage
   around_action :set_current_admin

app/controllers/sessions_controller.rb

@@ -26,8 +26,6 @@ class SessionsController < Devise::SessionsController
   before_action :load_recaptcha
   before_action :frontend_tracking_data, only: [:new]
 
-  around_action :set_current_context
-
   after_action :log_failed_login, if: :action_new_and_failed_login?
 
   helper_method :captcha_enabled?, :captcha_on_login_required?
@@ -307,13 +305,6 @@ class SessionsController < Devise::SessionsController
     # We want tracking data pushed to the frontend when the user is _in_ the control group
     frontend_experimentation_tracking_data(:signup_flow, 'start') unless experiment_enabled?(:signup_flow)
   end
-
-  def set_current_context(&block)
-    Gitlab::ApplicationContext.with_context(
-      user: -> { current_user },
-      caller_id: "#{self.class.name}##{action_name}",
-      &block)
-  end
 end
 
 SessionsController.prepend_if_ee('EE::SessionsController')

app/helpers/broadcast_messages_helper.rb

@@ -47,7 +47,15 @@ module BroadcastMessagesHelper
   end
 
   def render_broadcast_message(broadcast_message)
-    Banzai.render_field(broadcast_message, :message).html_safe
+    if Feature.enabled?(:broadcast_message_placeholders)
+      Banzai.render_and_post_process(broadcast_message.message, {
+        current_user: current_user,
+        skip_project_check: true,
+        broadcast_message_placeholders: true
+      }).html_safe
+    else
+      Banzai.render_field(broadcast_message, :message).html_safe
+    end
   end
 
   def broadcast_type_options

app/services/labels/transfer_service.rb

@@ -49,7 +49,7 @@ module Labels
       Label.joins(:issues)
         .where(
           issues: { project_id: project.id },
-          labels: { type: 'GroupLabel', group_id: old_group.id }
+          labels: { type: 'GroupLabel', group_id: old_group.self_and_ancestors }
         )
     end
     # rubocop: enable CodeReuse/ActiveRecord
@@ -59,14 +59,14 @@ module Labels
       Label.joins(:merge_requests)
         .where(
           merge_requests: { target_project_id: project.id },
-          labels: { type: 'GroupLabel', group_id: old_group.id }
+          labels: { type: 'GroupLabel', group_id: old_group.self_and_ancestors }
         )
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
     def find_or_create_label!(label)
       params    = label.attributes.slice('title', 'description', 'color')
-      new_label = FindOrCreateService.new(current_user, project, params).execute
+      new_label = FindOrCreateService.new(current_user, project, params.merge(include_ancestor_groups: true)).execute
 
       new_label.id
     end

changelogs/unreleased/210051-optimize-or-remove-ldap_users-counter.yml

+---
+title: Optimize members counters query performance in usage data
+merge_request: 27197
+author:
+type: performance

changelogs/unreleased/fix-duplicate-labels-when-moving-projects.yml

+---
+title: Fix duplicate labels when moving projects within the same ancestor group
+merge_request: 27261
+author:
+type: fixed

db/migrate/20200313123934_add_index_on_user_id_type_source_type_ldap_and_created_at_to_members.rb

+# frozen_string_literal: true
+
+class AddIndexOnUserIdTypeSourceTypeLdapAndCreatedAtToMembers < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'index_members_on_user_id_created_at'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :members, [:user_id, :created_at], where: "ldap = TRUE AND type = 'GroupMember' AND source_type = 'Namespace'", name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index :members, INDEX_NAME
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_03_12_163407) do
+ActiveRecord::Schema.define(version: 2020_03_13_123934) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -2442,6 +2442,7 @@ ActiveRecord::Schema.define(version: 2020_03_12_163407) do
     t.index ["invite_token"], name: "index_members_on_invite_token", unique: true
     t.index ["requested_at"], name: "index_members_on_requested_at"
     t.index ["source_id", "source_type"], name: "index_members_on_source_id_and_source_type"
+    t.index ["user_id", "created_at"], name: "index_members_on_user_id_created_at", where: "((ldap = true) AND ((type)::text = 'GroupMember'::text) AND ((source_type)::text = 'Namespace'::text))"
     t.index ["user_id"], name: "index_members_on_user_id"
   end
 

lib/banzai/filter/broadcast_message_placeholders_filter.rb

+# frozen_string_literal: true
+
+module Banzai
+  module Filter
+    # Replaces placeholders for broadcast messages with data from the current
+    # user or the instance.
+    class BroadcastMessagePlaceholdersFilter < HTML::Pipeline::Filter
+      def call
+        return doc unless context[:broadcast_message_placeholders]
+
+        doc.traverse { |node| replace_placeholders(node) }
+      end
+
+      private
+
+      def replace_placeholders(node)
+        if node.text? && !node.content.empty?
+          node.content = replace_content(node.content)
+        elsif href = link_href(node)
+          href.value = replace_content(href.value, url_safe_encoding: true)
+        end
+
+        node
+      end
+
+      def link_href(node)
+        node.element? &&
+          node.name == 'a' &&
+          node.attribute_nodes.find { |a| a.name == "href" }
+      end
+
+      def replace_content(content, url_safe_encoding: false)
+        placeholders.each do |placeholder, method|
+          regex = Regexp.new("{{#{placeholder}}}|#{CGI.escape("{{#{placeholder}}}")}")
+          value = url_safe_encoding ? CGI.escape(method.call.to_s) : method.call.to_s
+          content.gsub!(regex, value)
+        end
+
+        content
+      end
+
+      def placeholders
+        {
+          "email" => -> { current_user.try(:email) },
+          "name" => -> { current_user.try(:name) },
+          "user_id" => -> { current_user.try(:id) },
+          "username" => -> { current_user.try(:username) },
+          "instance_id" => -> { Gitlab::CurrentSettings.try(:uuid) }
+        }
+      end
+
+      def current_user
+        context[:current_user]
+      end
+    end
+  end
+end

lib/banzai/pipeline/post_process_pipeline.rb

@@ -8,7 +8,8 @@ module Banzai
       def self.filters
         @filters ||= FilterArray[
           *internal_link_filters,
-          Filter::AbsoluteLinkFilter
+          Filter::AbsoluteLinkFilter,
+          Filter::BroadcastMessagePlaceholdersFilter
         ]
       end
 

locale/gitlab.pot

@@ -217,6 +217,9 @@ msgstr ""
 msgid "%{authorsName}'s thread"
 msgstr ""
 
+msgid "%{buy_now_link_start}Buy now!%{link_end}"
+msgstr ""
+
 msgid "%{commit_author_link} authored %{commit_timeago}"
 msgstr ""
 
@@ -2412,6 +2415,12 @@ msgstr ""
 msgid "Ascending"
 msgstr ""
 
+msgid "Ask an admin to upload a new license to ensure uninterrupted service."
+msgstr ""
+
+msgid "Ask an admin to upload a new license to restore service."
+msgstr ""
+
 msgid "Ask your group maintainer to set up a group Runner."
 msgstr ""
 
@@ -16153,6 +16162,12 @@ msgstr ""
 msgid "Pushes"
 msgstr ""
 
+msgid "Pushing code and creation of issues and merge requests has been disabled."
+msgstr ""
+
+msgid "Pushing code and creation of issues and merge requests will be disabled on %{disabled_on}."
+msgstr ""
+
 msgid "PushoverService|%{user_name} deleted branch \"%{ref}\"."
 msgstr ""
 
@@ -21573,6 +21588,12 @@ msgstr ""
 msgid "Upload a certificate for your domain with all intermediates"
 msgstr ""
 
+msgid "Upload a new license in the admin area to ensure uninterrupted service."
+msgstr ""
+
+msgid "Upload a new license in the admin area to restore service."
+msgstr ""
+
 msgid "Upload a private key for your certificate"
 msgstr ""
 
@@ -23302,9 +23323,15 @@ msgstr ""
 msgid "Your issues will be imported in the background. Once finished, you'll get a confirmation email."
 msgstr ""
 
+msgid "Your license expired on %{expires_at}."
+msgstr ""
+
 msgid "Your license is valid from"
 msgstr ""
 
+msgid "Your license will expire in %{remaining_days}."
+msgstr ""
+
 msgid "Your message here"
 msgstr ""
 
@@ -23335,6 +23362,12 @@ msgstr ""
 msgid "Your request for access has been queued for review."
 msgstr ""
 
+msgid "Your trial license expired on %{expires_at}."
+msgstr ""
+
+msgid "Your trial license will expire in %{remaining_days}."
+msgstr ""
+
 msgid "Zoom meeting added"
 msgstr ""
 

spec/controllers/sessions_controller_spec.rb

@@ -497,13 +497,13 @@ describe SessionsController do
   end
 
   describe '#set_current_context' do
+    let_it_be(:user) { create(:user) }
+
     before do
       set_devise_mapping(context: @request)
     end
 
     context 'when signed in' do
-      let_it_be(:user) { create(:user) }
-
       before do
         sign_in(user)
       end
@@ -535,5 +535,25 @@ describe SessionsController do
         get :new
       end
     end
+
+    context 'when the user becomes locked' do
+      before do
+        user.update!(failed_attempts: User.maximum_attempts.pred)
+      end
+
+      it 'sets the caller_id in the context' do
+        allow_any_instance_of(User).to receive(:lock_access!).and_wrap_original do |m, *args|
+          expect(Labkit::Context.current.to_h)
+            .to include('meta.caller_id' => 'SessionsController#create')
+          expect(Labkit::Context.current.to_h)
+            .not_to include('meta.user')
+
+          m.call(*args)
+        end
+
+        post(:create,
+             params: { user: { login: user.username, password: user.password.succ } })
+      end
+    end
   end
 end

spec/features/broadcast_messages_spec.rb

@@ -57,4 +57,15 @@ describe 'Broadcast Messages' do
 
     it_behaves_like 'a dismissable Broadcast Messages'
   end
+
+  it 'renders broadcast message with placeholders' do
+    create(:broadcast_message, broadcast_type: 'notification', message: 'Hi {{name}}')
+
+    user = create(:user)
+    sign_in(user)
+
+    visit root_path
+
+    expect(page).to have_content "Hi #{user.name}"
+  end
 end

spec/helpers/broadcast_messages_helper_spec.rb

@@ -27,10 +27,11 @@ describe BroadcastMessagesHelper do
   end
 
   describe 'broadcast_message' do
+    let_it_be(:user) { create(:user) }
     let(:current_broadcast_message) { BroadcastMessage.new(message: 'Current Message') }
 
     before do
-      allow(helper).to receive(:current_user).and_return(create(:user))
+      allow(helper).to receive(:current_user).and_return(user)
     end
 
     it 'returns nil when no current message' do

spec/lib/banzai/filter/broadcast_message_placeholders_filter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Banzai::Filter::BroadcastMessagePlaceholdersFilter do
+  include FilterSpecHelper
+
+  subject { filter(text, current_user: user, broadcast_message_placeholders: true).to_html }
+
+  describe 'when current user is set' do
+    let_it_be(:user) { create(:user, email: "helloworld@example.com", name: "GitLab Tanunki :)") }
+
+    context 'replaces placeholder in text' do
+      let(:text) { 'Email: {{email}}' }
+
+      it { expect(subject).to eq("Email: #{user.email}") }
+    end
+
+    context 'replaces placeholder when they are in a link' do
+      let(:text) { '<a href="http://example.com?email={{email}}"">link</a>' }
+
+      it { expect(subject).to eq("<a href=\"http://example.com?email=helloworld%40example.com\">link</a>") }
+    end
+
+    context 'replaces placeholder when they are in an escaped link' do
+      let(:text) { '<a href="http://example.com?name=%7B%7Bname%7D%7D">link</a>' }
+
+      it { expect(subject).to eq("<a href=\"http://example.com?name=GitLab+Tanunki+%3A%29\">link</a>") }
+    end
+
+    context 'works with empty text' do
+      let(:text) {" "}
+
+      it { expect(subject).to eq(" ") }
+    end
+
+    context 'replaces multiple placeholders in a given text' do
+      let(:text) { "{{email}} {{name}}" }
+
+      it { expect(subject).to eq("#{user.email} #{user.name}") }
+    end
+
+    context 'available placeholders' do
+      context 'replaces the email of the user' do
+        let(:text) { "{{email}}"}
+
+        it { expect(subject).to eq(user.email) }
+      end
+
+      context 'replaces the name of the user' do
+        let(:text) { "{{name}}"}
+
+        it { expect(subject).to eq(user.name) }
+      end
+
+      context 'replaces the ID of the user' do
+        let(:text) { "{{user_id}}" }
+
+        it { expect(subject).to eq(user.id.to_s) }
+      end
+
+      context 'replaces the username of the user' do
+        let(:text) { "{{username}}" }
+
+        it { expect(subject).to eq(user.username) }
+      end
+
+      context 'replaces the instance_id' do
+        before do
+          stub_application_setting(uuid: '123')
+        end
+
+        let(:text) { "{{instance_id}}" }
+
+        it { expect(subject).to eq(Gitlab::CurrentSettings.uuid) }
+      end
+    end
+  end
+
+  describe 'when there is no current user set' do
+    let(:user) { nil }
+
+    context 'replaces placeholder with empty string' do
+      let(:text) { "Email: {{email}}" }
+
+      it { expect(subject).to eq("Email: ") }
+    end
+  end
+end

spec/services/labels/transfer_service_spec.rb

@@ -4,65 +4,101 @@ require 'spec_helper'
 
 describe Labels::TransferService do
   describe '#execute' do
-    let(:user)    { create(:admin) }
-    let(:group_1) { create(:group) }
-    let(:group_2) { create(:group) }
-    let(:group_3) { create(:group) }
-    let(:project_1) { create(:project, namespace: group_2) }
-    let(:project_2) { create(:project, namespace: group_3) }
-    let(:project_3) { create(:project, namespace: group_1) }
-
-    let(:group_label_1) { create(:group_label, group: group_1, name: 'Group Label 1') }
-    let(:group_label_2) { create(:group_label, group: group_1, name: 'Group Label 2') }
-    let(:group_label_3) { create(:group_label, group: group_1, name: 'Group Label 3') }
-    let(:group_label_4) { create(:group_label, group: group_2, name: 'Group Label 4') }
-    let(:group_label_5) { create(:group_label, group: group_3, name: 'Group Label 5') }
-    let(:project_label_1) { create(:label, project: project_1, name: 'Project Label 1') }
-
-    subject(:service) { described_class.new(user, group_1, project_1) }
-
-    before do
-      create(:labeled_issue, project: project_1, labels: [group_label_1])
-      create(:labeled_issue, project: project_1, labels: [group_label_4])
-      create(:labeled_issue, project: project_1, labels: [project_label_1])
-      create(:labeled_issue, project: project_2, labels: [group_label_5])
-      create(:labeled_issue, project: project_3, labels: [group_label_1])
-      create(:labeled_merge_request, source_project: project_1, labels: [group_label_1, group_label_2])
-      create(:labeled_merge_request, source_project: project_2, labels: [group_label_5])
+    let_it_be(:user) { create(:admin) }
+
+    let_it_be(:old_group_ancestor) { create(:group) }
+    let_it_be(:old_group) { create(:group, parent: old_group_ancestor) }
+
+    let_it_be(:new_group) { create(:group) }
+
+    let_it_be(:project) { create(:project, :repository, group: new_group) }
+
+    subject(:service) { described_class.new(user, old_group, project) }
+
+    it 'recreates missing group labels at project level and assigns them to the issuables' do
+      old_group_label_1 = create(:group_label, group: old_group)
+      old_group_label_2 = create(:group_label, group: old_group)
+
+      labeled_issue = create(:labeled_issue, project: project, labels: [old_group_label_1])
+      labeled_merge_request = create(:labeled_merge_request, source_project: project, labels: [old_group_label_2])
+
+      expect { service.execute }.to change(project.labels, :count).by(2)
+      expect(labeled_issue.reload.labels).to contain_exactly(project.labels.find_by_title(old_group_label_1.title))
+      expect(labeled_merge_request.reload.labels).to contain_exactly(project.labels.find_by_title(old_group_label_2.title))
     end
 
-    it 'recreates the missing group labels at project level' do
-      expect { service.execute }.to change(project_1.labels, :count).by(2)
+    it 'recreates missing ancestor group labels at project level and assigns them to the issuables' do
+      old_group_ancestor_label_1 = create(:group_label, group: old_group_ancestor)
+      old_group_ancestor_label_2 = create(:group_label, group: old_group_ancestor)
+
+      labeled_issue = create(:labeled_issue, project: project, labels: [old_group_ancestor_label_1])
+      labeled_merge_request = create(:labeled_merge_request, source_project: project, labels: [old_group_ancestor_label_2])
+
+      expect { service.execute }.to change(project.labels, :count).by(2)
+      expect(labeled_issue.reload.labels).to contain_exactly(project.labels.find_by_title(old_group_ancestor_label_1.title))
+      expect(labeled_merge_request.reload.labels).to contain_exactly(project.labels.find_by_title(old_group_ancestor_label_2.title))
     end
 
     it 'recreates label priorities related to the missing group labels' do
-      create(:label_priority, project: project_1, label: group_label_1, priority: 1)
+      old_group_label = create(:group_label, group: old_group)
+      create(:labeled_issue, project: project, labels: [old_group_label])
+      create(:label_priority, project: project, label: old_group_label, priority: 1)
 
       service.execute
 
-      new_project_label = project_1.labels.find_by(title: group_label_1.title)
-      expect(new_project_label.id).not_to eq group_label_1.id
+      new_project_label = project.labels.find_by(title: old_group_label.title)
+      expect(new_project_label.id).not_to eq old_group_label.id
       expect(new_project_label.priorities).not_to be_empty
     end
 
     it 'does not recreate missing group labels that are not applied to issues or merge requests' do
+      old_group_label = create(:group_label, group: old_group)
+
       service.execute
 
-      expect(project_1.labels.where(title: group_label_3.title)).to be_empty
+      expect(project.labels.where(title: old_group_label.title)).to be_empty
     end
 
     it 'does not recreate missing group labels that already exist in the project group' do
+      old_group_label = create(:group_label, group: old_group)
+      labeled_issue = create(:labeled_issue, project: project, labels: [old_group_label])
+
+      new_group_label = create(:group_label, group: new_group, title: old_group_label.title)
+
       service.execute
 
-      expect(project_1.labels.where(title: group_label_4.title)).to be_empty
+      expect(project.labels.where(title: old_group_label.title)).to be_empty
+      expect(labeled_issue.reload.labels).to contain_exactly(new_group_label)
     end
 
     it 'updates only label links in the given project' do
+      old_group_label = create(:group_label, group: old_group)
+      other_project = create(:project, group: old_group)
+
+      labeled_issue = create(:labeled_issue, project: project, labels: [old_group_label])
+      other_project_labeled_issue = create(:labeled_issue, project: other_project, labels: [old_group_label])
+
       service.execute
 
-      targets = LabelLink.where(label_id: group_label_1.id).map(&:target)
+      expect(labeled_issue.reload.labels).not_to include(old_group_label)
+      expect(other_project_labeled_issue.reload.labels).to contain_exactly(old_group_label)
+    end
+
+    context 'when moving within the same ancestor group' do
+      let(:other_subgroup) { create(:group, parent: old_group_ancestor) }
+      let(:project) { create(:project, :repository, group: other_subgroup) }
+
+      it 'does not recreate ancestor group labels' do
+        old_group_ancestor_label_1 = create(:group_label, group: old_group_ancestor)
+        old_group_ancestor_label_2 = create(:group_label, group: old_group_ancestor)
+
+        labeled_issue = create(:labeled_issue, project: project, labels: [old_group_ancestor_label_1])
+        labeled_merge_request = create(:labeled_merge_request, source_project: project, labels: [old_group_ancestor_label_2])
 
-      expect(targets).to eq(project_3.issues)
+        expect { service.execute }.not_to change(project.labels, :count)
+        expect(labeled_issue.reload.labels).to contain_exactly(old_group_ancestor_label_1)
+        expect(labeled_merge_request.reload.labels).to contain_exactly(old_group_ancestor_label_2)
+      end
     end
   end
 end