Automatic merge of gitlab-org/gitlab master

app/models/ci/pipeline.rb

@@ -831,9 +831,8 @@ module Ci
     end
 
     def execute_hooks
-      data = pipeline_data
-      project.execute_hooks(data, :pipeline_hooks)
-      project.execute_services(data, :pipeline_hooks)
+      project.execute_hooks(pipeline_data, :pipeline_hooks) if project.has_active_hooks?(:pipeline_hooks)
+      project.execute_services(pipeline_data, :pipeline_hooks) if project.has_active_services?(:pipeline_hooks)
     end
 
     # All the merge requests for which the current pipeline runs/ran against
@@ -1159,7 +1158,9 @@ module Ci
     end
 
     def pipeline_data
-      Gitlab::DataBuilder::Pipeline.build(self)
+      strong_memoize(:pipeline_data) do
+        Gitlab::DataBuilder::Pipeline.build(self)
+      end
     end
 
     def merge_request_diff_sha

changelogs/unreleased/cablett-247867-epics-relative-position.yml

+---
+title: Add Epic Board Position model to store relative positioning of epics on a board
+merge_request: 48120
+author:
+type: added

changelogs/unreleased/sh-avoid-build-hooks-data.yml

+---
+title: Reduce SQL queries when no pipeline hooks are active
+merge_request: 49186
+author:
+type: performance

db/migrate/20201202003042_add_epic_board_positions.rb

+# frozen_string_literal: true
+
+class AddEpicBoardPositions < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      create_table :boards_epic_board_positions do |t|
+        t.references :epic_board, foreign_key: { to_table: :boards_epic_boards, on_delete: :cascade }, null: false, index: false
+        t.references :epic, foreign_key: { on_delete: :cascade }, null: false, index: true
+        t.integer :relative_position
+
+        t.timestamps_with_timezone null: false
+
+        t.index [:epic_board_id, :epic_id], unique: true, name: :index_boards_epic_board_positions_on_epic_board_id_and_epic_id
+      end
+    end
+  end
+
+  def down
+    with_lock_retries do
+      drop_table :boards_epic_board_positions
+    end
+  end
+end

db/schema_migrations/20201202003042

+779effb1db70aa8b9a24942ec3e0681064c01b69ee4731f82477c54361a670b0
\ No newline at end of file

db/structure.sql

@@ -9864,6 +9864,24 @@ CREATE SEQUENCE boards_epic_board_labels_id_seq
 
 ALTER SEQUENCE boards_epic_board_labels_id_seq OWNED BY boards_epic_board_labels.id;
 
+CREATE TABLE boards_epic_board_positions (
+    id bigint NOT NULL,
+    epic_board_id bigint NOT NULL,
+    epic_id bigint NOT NULL,
+    relative_position integer,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL
+);
+
+CREATE SEQUENCE boards_epic_board_positions_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE boards_epic_board_positions_id_seq OWNED BY boards_epic_board_positions.id;
+
 CREATE TABLE boards_epic_boards (
     id bigint NOT NULL,
     hide_backlog_list boolean DEFAULT false NOT NULL,
@@ -17920,6 +17938,8 @@ ALTER TABLE ONLY boards ALTER COLUMN id SET DEFAULT nextval('boards_id_seq'::reg
 
 ALTER TABLE ONLY boards_epic_board_labels ALTER COLUMN id SET DEFAULT nextval('boards_epic_board_labels_id_seq'::regclass);
 
+ALTER TABLE ONLY boards_epic_board_positions ALTER COLUMN id SET DEFAULT nextval('boards_epic_board_positions_id_seq'::regclass);
+
 ALTER TABLE ONLY boards_epic_boards ALTER COLUMN id SET DEFAULT nextval('boards_epic_boards_id_seq'::regclass);
 
 ALTER TABLE ONLY boards_epic_user_preferences ALTER COLUMN id SET DEFAULT nextval('boards_epic_user_preferences_id_seq'::regclass);
@@ -18953,6 +18973,9 @@ ALTER TABLE ONLY board_user_preferences
 ALTER TABLE ONLY boards_epic_board_labels
     ADD CONSTRAINT boards_epic_board_labels_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY boards_epic_board_positions
+    ADD CONSTRAINT boards_epic_board_positions_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY boards_epic_boards
     ADD CONSTRAINT boards_epic_boards_pkey PRIMARY KEY (id);
 
@@ -20582,6 +20605,10 @@ CREATE INDEX index_boards_epic_board_labels_on_epic_board_id ON boards_epic_boar
 
 CREATE INDEX index_boards_epic_board_labels_on_label_id ON boards_epic_board_labels USING btree (label_id);
 
+CREATE UNIQUE INDEX index_boards_epic_board_positions_on_epic_board_id_and_epic_id ON boards_epic_board_positions USING btree (epic_board_id, epic_id);
+
+CREATE INDEX index_boards_epic_board_positions_on_epic_id ON boards_epic_board_positions USING btree (epic_id);
+
 CREATE INDEX index_boards_epic_boards_on_group_id ON boards_epic_boards USING btree (group_id);
 
 CREATE INDEX index_boards_epic_user_preferences_on_board_id ON boards_epic_user_preferences USING btree (board_id);
@@ -23844,6 +23871,9 @@ ALTER TABLE ONLY approver_groups
 ALTER TABLE ONLY packages_tags
     ADD CONSTRAINT fk_rails_1dfc868911 FOREIGN KEY (package_id) REFERENCES packages_packages(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY boards_epic_board_positions
+    ADD CONSTRAINT fk_rails_1ecfd9f2de FOREIGN KEY (epic_id) REFERENCES epics(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY geo_repository_created_events
     ADD CONSTRAINT fk_rails_1f49e46a61 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 
@@ -24771,6 +24801,9 @@ ALTER TABLE ONLY gpg_signatures
 ALTER TABLE ONLY board_group_recent_visits
     ADD CONSTRAINT fk_rails_ca04c38720 FOREIGN KEY (board_id) REFERENCES boards(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY boards_epic_board_positions
+    ADD CONSTRAINT fk_rails_cb4563dd6e FOREIGN KEY (epic_board_id) REFERENCES boards_epic_boards(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY vulnerability_finding_links
     ADD CONSTRAINT fk_rails_cbdfde27ce FOREIGN KEY (vulnerability_occurrence_id) REFERENCES vulnerability_occurrences(id) ON DELETE CASCADE;
 

ee/app/models/boards/epic_board.rb

@@ -4,6 +4,7 @@ module Boards
   class EpicBoard < ApplicationRecord
     belongs_to :group, optional: false, inverse_of: :epic_boards
     has_many :epic_board_labels, foreign_key: :epic_board_id, inverse_of: :epic_board
+    has_many :epic_board_positions, foreign_key: :epic_board_id, inverse_of: :epic_board
 
     validates :name, length: { maximum: 255 }
   end

ee/app/models/boards/epic_board_position.rb

+# frozen_string_literal: true
+
+module Boards
+  class EpicBoardPosition < ApplicationRecord
+    include RelativePositioning
+
+    belongs_to :epic_board, optional: false, inverse_of: :epic_board_positions
+    belongs_to :epic, optional: false
+
+    alias_attribute :parent, :epic_board
+    validates :epic, uniqueness: { scope: :epic_board_id }
+
+    scope :order_relative_position, -> do
+      reorder('relative_position ASC', 'id DESC')
+    end
+
+    def self.relative_positioning_query_base(position)
+      where(epic_board_id: position.epic_board_id)
+    end
+
+    def self.relative_positioning_parent_column
+      :epic_board_id
+    end
+  end
+end

ee/app/models/ee/vulnerability.rb

@@ -139,19 +139,6 @@ module EE
         findings.first
       end
 
-      # TODO: Remove this attribute reader overrides with #262112
-      def dismissed_at
-        return unless dismissed?
-
-        super || fallback_dismissal_feedback&.created_at
-      end
-
-      def dismissed_by_id
-        return unless dismissed?
-
-        super || fallback_dismissal_feedback&.author_id
-      end
-
       def resource_parent
         project
       end
@@ -179,15 +166,6 @@ module EE
       def user_notes_count_service
         @user_notes_count_service ||= ::Vulnerabilities::UserNotesCountService.new(self) # rubocop: disable CodeReuse/ServiceClass
       end
-
-      # TODO: Remove this with #262112
-      def fallback_dismissal_feedback
-        strong_memoize(:fallback_dismissal_feedback) do
-          ::Gitlab::AppJsonLogger.warn(message: 'Fallback dismissal_feedback has been called!', vulnerability_id: id)
-
-          finding&.dismissal_feedback
-        end
-      end
     end
 
     class_methods do

ee/changelogs/unreleased/banner-future-date-license.yml

+---
+title: 'Do not display renewal banner if future dated license is applied'
+merge_request: 48283
+author:
+type: changed

ee/lib/gitlab/expiring_subscription_message.rb

@@ -120,7 +120,7 @@ module Gitlab
     end
 
     def require_notification?
-      return false if expiring_auto_renew?
+      return false if expiring_auto_renew? || ::License.future_dated.present?
 
       auto_renew_choice_exists? && expired_subscribable_within_notification_window?
     end

ee/spec/factories/epic_board_positions.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :epic_board_position, class: 'Boards::EpicBoardPosition' do
+    epic
+    epic_board
+    relative_position { RelativePositioning::START_POSITION }
+  end
+end

ee/spec/factories/epic_boards.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :epic_board, class: 'Boards::EpicBoard' do
+    name
+    group
+  end
+end

ee/spec/helpers/ee/subscribable_banner_helper_spec.rb

@@ -77,6 +77,19 @@ RSpec.describe EE::SubscribableBannerHelper do
           expect(subject).to eq(license)
         end
       end
+
+      context 'with a future dated license' do
+        let(:gl_license) { build(:gitlab_license, starts_at: Date.current + 1.month) }
+
+        before do
+          allow(::Gitlab::CurrentSettings).to receive(:should_check_namespace_plan?).and_return(true)
+        end
+
+        it 'returns the current license' do
+          allow(License).to receive(:current).and_return(license)
+          expect(subject).to eq(license)
+        end
+      end
     end
   end
 

ee/spec/lib/gitlab/expiring_subscription_message_spec.rb

@@ -162,6 +162,16 @@ RSpec.describe Gitlab::ExpiringSubscriptionMessage do
               end
             end
 
+            context 'when a future dated license is applied' do
+              before do
+                create(:license, created_at: Time.current, data: build(:gitlab_license, starts_at: expired_date, expires_at: Date.current + 13.months).export)
+              end
+
+              it 'returns nil' do
+                expect(subject).to be nil
+              end
+            end
+
             context 'with namespace' do
               let(:namespace) { double(:namespace, name: 'No Limit Records') }
 

ee/spec/models/boards/epic_board_position_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Boards::EpicBoardPosition do
+  let_it_be(:epic) { create(:epic) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:epic_board) { create(:epic_board, group: group) }
+  let_it_be(:epic_board_position) { create(:epic_board_position, epic: epic, epic_board: epic_board) }
+
+  describe 'associations' do
+    subject { build(:epic_board_position) }
+
+    it { is_expected.to belong_to(:epic).required }
+    it { is_expected.to belong_to(:epic_board).required.inverse_of(:epic_board_positions) }
+  end
+
+  describe 'validations' do
+    subject { build(:epic_board_position) }
+
+    specify { expect(subject).to be_valid }
+
+    it 'is valid with nil relative position' do
+      subject.relative_position = nil
+
+      expect(subject).to be_valid
+    end
+
+    it 'disallows a record with same epic and board' do
+      expect(build(:epic_board_position, epic: epic, epic_board: epic_board)).not_to be_valid
+    end
+  end
+
+  describe 'scopes' do
+    describe '.order_relative_position' do
+      let(:first) { epic_board_position }
+      let!(:second) { create(:epic_board_position, epic_board: epic_board, relative_position: RelativePositioning::START_POSITION + 7 ) }
+
+      it 'returns epic_board_positions in order' do
+        expect(described_class.order_relative_position).to eq([first, second])
+      end
+    end
+  end
+
+  context 'relative positioning' do
+    let_it_be(:positioning_group) { create(:group) }
+    let_it_be(:positioning_board) { create(:epic_board, group: positioning_group) }
+
+    it_behaves_like "a class that supports relative positioning" do
+      let(:factory) { :epic_board_position }
+      let(:default_params) { { parent: positioning_board } }
+    end
+  end
+end

ee/spec/models/boards/epic_board_spec.rb

@@ -6,6 +6,7 @@ RSpec.describe Boards::EpicBoard do
   describe 'associations' do
     it { is_expected.to belong_to(:group).required.inverse_of(:epic_boards) }
     it { is_expected.to have_many(:epic_board_labels).inverse_of(:epic_board) }
+    it { is_expected.to have_many(:epic_board_positions).inverse_of(:epic_board) }
   end
 
   describe 'validations' do

ee/spec/models/ee/vulnerability_spec.rb

@@ -674,138 +674,6 @@ RSpec.describe Vulnerability do
     end
   end
 
-  describe '#dismissed_at' do
-    let_it_be(:project) { create(:project) }
-    let_it_be(:finding) do
-      create(
-        :vulnerabilities_finding,
-        report_type: :dependency_scanning,
-        project: project
-      )
-    end
-
-    let(:vulnerability) { create(:vulnerability, findings: [finding]) }
-    let(:feedback_created_at) { -2.days.from_now }
-    let!(:dismissal_feedback) do
-      create(
-        :vulnerability_feedback,
-        :dependency_scanning,
-        :dismissal,
-        project: project,
-        project_fingerprint: finding.project_fingerprint,
-        created_at: feedback_created_at
-      )
-    end
-
-    subject(:dismissed_at) { vulnerability.dismissed_at }
-
-    around do |example|
-      freeze_time { example.run }
-    end
-
-    context 'when the vulnerability is not dismissed' do
-      before do
-        vulnerability.update_attribute(:dismissed_at, Time.current)
-      end
-
-      it { is_expected.to be_nil }
-    end
-
-    context 'when the vulnerability is dismissed' do
-      before do
-        vulnerability.dismissed!
-      end
-
-      context 'when the `dismissed_at` exists' do
-        let(:vulnerability_dismissed_at) { -1.day.from_now }
-
-        before do
-          vulnerability.update_attribute(:dismissed_at, vulnerability_dismissed_at)
-        end
-
-        it { is_expected.to eq(vulnerability_dismissed_at) }
-      end
-
-      context 'when the `dismissed_at` does not exist' do
-        before do
-          allow(::Gitlab::AppJsonLogger).to receive(:warn)
-        end
-
-        it { is_expected.to eq(feedback_created_at) }
-
-        it 'puts a warning log' do
-          dismissed_at
-
-          expect(::Gitlab::AppJsonLogger).to have_received(:warn)
-        end
-      end
-    end
-  end
-
-  describe '#dismissed_by_id' do
-    let_it_be(:user_1) { create(:user) }
-    let_it_be(:user_2) { create(:user) }
-    let_it_be(:project) { create(:project) }
-    let_it_be(:occurrence) do
-      create(
-        :vulnerabilities_finding,
-        report_type: :dependency_scanning,
-        project: project
-      )
-    end
-
-    let_it_be(:dismissal_feedback) do
-      create(
-        :vulnerability_feedback,
-        :dependency_scanning,
-        :dismissal,
-        project: project,
-        project_fingerprint: occurrence.project_fingerprint,
-        author: user_1
-      )
-    end
-
-    let(:vulnerability) { create(:vulnerability, findings: [occurrence]) }
-
-    subject(:dismissed_by_id) { vulnerability.dismissed_by_id }
-
-    context 'when the vulnerability is not dismissed' do
-      before do
-        vulnerability.update_attribute(:dismissed_by_id, user_1.id)
-      end
-
-      it { is_expected.to be_nil }
-    end
-
-    context 'when the vulnerability is dismissed' do
-      before do
-        vulnerability.dismissed!
-      end
-
-      context 'when the `dismissed_by_id` exists' do
-        before do
-          vulnerability.update_attribute(:dismissed_by_id, user_2.id)
-        end
-
-        it { is_expected.to eq(user_2.id) }
-      end
-
-      context 'when the `dismissed_by_id` does not exist' do
-        before do
-          allow(::Gitlab::AppJsonLogger).to receive(:warn)
-        end
-
-        it { is_expected.to eq(user_1.id) }
-
-        it 'puts a warning log' do
-          dismissed_by_id
-
-          expect(::Gitlab::AppJsonLogger).to have_received(:warn)
-        end
-      end
-    end
-  end
-
   describe '#user_notes_count' do
     let_it_be(:vulnerability) { create(:vulnerability) }
 

rubocop/rubocop-migrations.yml

@@ -37,6 +37,7 @@ Migration/UpdateLargeTable:
     - :todos
     - :users
     - :user_preferences
+    - :user_details
     - :web_hook_logs
   DeniedMethods:
     - :change_column_type_concurrently

spec/models/ci/pipeline_spec.rb

@@ -2578,6 +2578,14 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
           it 'receives a pending event once' do
             expect(WebMock).to have_requested_pipeline_hook('pending').once
           end
+
+          it 'builds hook data once' do
+            create(:pipelines_email_service, project: project)
+
+            expect(Gitlab::DataBuilder::Pipeline).to receive(:build).once.and_call_original
+
+            pipeline.execute_hooks
+          end
         end
 
         context 'when build is run' do
@@ -2639,6 +2647,12 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
       it 'did not execute pipeline_hook after touched' do
         expect(WebMock).not_to have_requested(:post, hook.url)
       end
+
+      it 'does not build hook data' do
+        expect(Gitlab::DataBuilder::Pipeline).not_to receive(:build)
+
+        pipeline.execute_hooks
+      end
     end
 
     def create_build(name, stage_idx)