Add support for event creation on designs

Add methods for adding design events to the EventCreateService, and
uses these methods in the design management services.

A new spec support class is added (MultiChangeValue) to help testing
several change values at once, without having to perform the action
multiple times.

Deletion in particular is made efficient by ensuring that we perform
just a single insert when creating events.

app/models/design_management/design.rb

@@ -20,6 +20,8 @@ module DesignManagement
     has_many :notes, as: :noteable, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
     has_many :user_mentions, class_name: 'DesignUserMention', dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
 
+    has_many :events, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
+
     validates :project, :filename, presence: true
     validates :issue, presence: true, unless: :importing?
     validates :filename, uniqueness: { scope: :issue_id }, length: { maximum: 255 }
@@ -189,6 +191,11 @@ module DesignManagement
     alias_method :after_note_created,   :after_note_changed
     alias_method :after_note_destroyed, :after_note_changed
 
+    # Part of the interface of objects we can create events about
+    def resource_parent
+      project
+    end
+
     private
 
     def head_version

app/services/design_management/delete_designs_service.rb

@@ -15,6 +15,7 @@ module DesignManagement
       return error('Forbidden!') unless can_delete_designs?
 
       version = delete_designs!
+      EventCreateService.new.destroy_designs(designs, current_user)
 
       success(version: version)
     end
@@ -48,7 +49,9 @@ module DesignManagement
     end
 
     def design_action(design)
-      on_success { counter.count(:delete) }
+      on_success do
+        counter.count(:delete)
+      end
 
       DesignManagement::DesignAction.new(design, :delete)
     end

app/services/design_management/save_designs_service.rb

@@ -20,6 +20,7 @@ module DesignManagement
       uploaded_designs, version = upload_designs!
       skipped_designs = designs - uploaded_designs
 
+      create_events
       success({ designs: uploaded_designs, version: version, skipped_designs: skipped_designs })
     rescue ::ActiveRecord::RecordInvalid => e
       error(e.message)
@@ -47,7 +48,7 @@ module DesignManagement
     end
 
     def build_actions
-      files.zip(designs).flat_map do |(file, design)|
+      @actions ||= files.zip(designs).flat_map do |(file, design)|
         Array.wrap(build_design_action(file, design))
       end
     end
@@ -57,7 +58,9 @@ module DesignManagement
       return if design_unchanged?(design, content)
 
       action = new_file?(design) ? :create : :update
-      on_success { ::Gitlab::UsageDataCounters::DesignsCounter.count(action) }
+      on_success do
+        ::Gitlab::UsageDataCounters::DesignsCounter.count(action)
+      end
 
       DesignManagement::DesignAction.new(design, action, content)
     end
@@ -67,6 +70,16 @@ module DesignManagement
       content == existing_blobs[design]&.data
     end
 
+    def create_events
+      by_action = @actions.group_by(&:action).transform_values { |grp| grp.map(&:design) }
+
+      event_create_service.save_designs(current_user, **by_action)
+    end
+
+    def event_create_service
+      @event_create_service ||= EventCreateService.new
+    end
+
     def commit_message
       <<~MSG
       Updated #{files.size} #{'designs'.pluralize(files.size)}

app/services/event_create_service.rb

@@ -96,6 +96,29 @@ class EventCreateService
     create_push_event(BulkPushEventPayloadService, project, current_user, push_data)
   end
 
+  def save_designs(current_user, create: [], update: [])
+    created = create.group_by(&:project).flat_map do |project, designs|
+      Feature.enabled?(:design_activity_events, project) ? designs : []
+    end.to_set
+    updated = update.group_by(&:project).flat_map do |project, designs|
+      Feature.enabled?(:design_activity_events, project) ? designs : []
+    end.to_set
+    return [] if created.empty? && updated.empty?
+
+    records = created.zip([:created].cycle) + updated.zip([:updated].cycle)
+
+    create_record_events(records, current_user)
+  end
+
+  def destroy_designs(designs, current_user)
+    designs = designs.select do |design|
+      Feature.enabled?(:design_activity_events, design.project)
+    end
+    return [] unless designs.present?
+
+    create_record_events(designs.zip([:destroyed].cycle), current_user)
+  end
+
   # Create a new wiki page event
   #
   # @param [WikiPage::Meta] wiki_page_meta The event target
@@ -134,7 +157,32 @@ class EventCreateService
   end
 
   def create_record_event(record, current_user, status)
-    create_event(record.resource_parent, current_user, status, target_id: record.id, target_type: record.class.name)
+    create_event(record.resource_parent, current_user, status,
+                 target_id: record.id, target_type: record.class.name)
+  end
+
+  # If creating several events, this method will insert them all in a single
+  # statement
+  #
+  # @param [[Eventable, Symbol]] a list of pairs of records and a valid status
+  # @param [User] the author of the event
+  def create_record_events(pairs, current_user)
+    base_attrs = {
+      created_at: Time.now.utc,
+      updated_at: Time.now.utc,
+      author_id: current_user.id
+    }
+
+    attribute_sets = pairs.map do |record, status|
+      action = Event.actions[status]
+      raise IllegalActionError, "#{status} is not a valid status" if action.nil?
+
+      parent_attrs(record.resource_parent)
+        .merge(base_attrs)
+        .merge(action: action, target_id: record.id, target_type: record.class.name)
+    end
+
+    Event.insert_all(attribute_sets, returning: %w[id])
   end
 
   def create_push_event(service_class, project, current_user, push_data)
@@ -160,16 +208,22 @@ class EventCreateService
       action: status,
       author_id: current_user.id
     )
+    attributes.merge!(parent_attrs(resource_parent))
 
+    Event.create!(attributes)
+  end
+
+  def parent_attrs(resource_parent)
     resource_parent_attr = case resource_parent
                            when Project
-                             :project
+                             :project_id
                            when Group
-                             :group
+                             :group_id
                            end
-    attributes[resource_parent_attr] = resource_parent if resource_parent_attr
 
-    Event.create!(attributes)
+    return {} unless resource_parent_attr
+
+    { resource_parent_attr => resource_parent.id }
   end
 
   def create_resource_event(issuable, current_user, status)

spec/services/design_management/delete_designs_service_spec.rb

@@ -56,6 +56,10 @@ describe DesignManagement::DeleteDesignsService do
       let(:enabled) { false }
 
       it_behaves_like "a service error"
+
+      it 'does not create any events in the activity stream' do
+        expect { run_service rescue nil }.not_to change { Event.count }
+      end
     end
 
     context "when the feature is available" do
@@ -72,7 +76,9 @@ describe DesignManagement::DeleteDesignsService do
 
         it 'does not log any events' do
           counter = ::Gitlab::UsageDataCounters::DesignsCounter
-          expect { run_service rescue nil }.not_to change { counter.totals }
+
+          expect { run_service rescue nil }
+            .not_to change { [counter.totals, Event.count] }
         end
       end
 
@@ -92,6 +98,12 @@ describe DesignManagement::DeleteDesignsService do
           expect { run_service }.to change { counter.read(:delete) }.by(1)
         end
 
+        it 'creates an event in the activity stream' do
+          expect { run_service }
+            .to change { Event.count }.by(1)
+            .and change { Event.destroyed_action.for_design.count }.by(1)
+        end
+
         it 'informs the new-version-worker' do
           expect(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer)
 
@@ -129,14 +141,14 @@ describe DesignManagement::DeleteDesignsService do
 
         let!(:designs) { create_designs(2) }
 
-        it 'removes those designs' do
+        it 'makes the correct changes' do
+          counter = ::Gitlab::UsageDataCounters::DesignsCounter
+
           expect { run_service }
             .to change { issue.designs.current.count }.from(3).to(1)
-        end
-
-        it 'logs the correct number of deletion events' do
-          counter = ::Gitlab::UsageDataCounters::DesignsCounter
-          expect { run_service }.to change { counter.read(:delete) }.by(2)
+            .and change { counter.read(:delete) }.by(2)
+            .and change { Event.count }.by(2)
+            .and change { Event.destroyed_action.for_design.count }.by(2)
         end
 
         it_behaves_like "a success"

spec/services/design_management/save_designs_service_spec.rb

@@ -65,6 +65,10 @@ describe DesignManagement::SaveDesignsService do
       end
 
       it_behaves_like 'a service error'
+
+      it 'does not create an event in the activity stream' do
+        expect { run_service }.not_to change { Event.count }
+      end
     end
 
     context 'when the feature is available' do
@@ -89,6 +93,12 @@ describe DesignManagement::SaveDesignsService do
         expect { run_service }.to change { counter.read(:create) }.by(1)
       end
 
+      it 'creates an event in the activity stream' do
+        expect { run_service }
+          .to change { Event.count }.by(1)
+          .and change { Event.for_design.created_action.count }.by(1)
+      end
+
       it 'creates a commit in the repository' do
         run_service
 
@@ -166,9 +176,12 @@ describe DesignManagement::SaveDesignsService do
           expect(updated_designs.first.versions.size).to eq(2)
         end
 
-        it 'increments the update counter' do
+        it 'records the correct events' do
           counter = Gitlab::UsageDataCounters::DesignsCounter
-          expect { run_service }.to change { counter.read(:update) }.by 1
+          expect { run_service }
+            .to change { counter.read(:update) }.by(1)
+            .and change { Event.count }.by(1)
+            .and change { Event.for_design.updated_action.count }.by(1)
         end
 
         context 'when uploading a new design' do
@@ -217,6 +230,14 @@ describe DesignManagement::SaveDesignsService do
             .and change { counter.read(:update) }.by(1)
         end
 
+        it 'creates the correct activity stream events' do
+          expect { run_service }
+            .to change { Event.count }.by(2)
+            .and change { Event.for_design.count }.by(2)
+            .and change { Event.created_action.count }.by(1)
+            .and change { Event.updated_action.count }.by(1)
+        end
+
         it 'creates a single commit' do
           commit_count = -> do
             design_repository.expire_all_method_caches

spec/services/event_create_service_spec.rb

@@ -5,6 +5,9 @@ require 'spec_helper'
 describe EventCreateService do
   let(:service) { described_class.new }
 
+  let_it_be(:user, reload: true) { create :user }
+  let_it_be(:project) { create(:project) }
+
   describe 'Issues' do
     describe '#open_issue' do
       let(:issue) { create(:issue) }
@@ -87,8 +90,6 @@ describe EventCreateService do
   end
 
   describe 'Milestone' do
-    let(:user) { create :user }
-
     describe '#open_milestone' do
       let(:milestone) { create(:milestone) }
 
@@ -210,9 +211,6 @@ describe EventCreateService do
   end
 
   describe '#push', :clean_gitlab_redis_shared_state do
-    let(:project) { create(:project) }
-    let(:user) { create(:user) }
-
     let(:push_data) do
       {
         commits: [
@@ -234,9 +232,6 @@ describe EventCreateService do
   end
 
   describe '#bulk_push', :clean_gitlab_redis_shared_state do
-    let(:project) { create(:project) }
-    let(:user) { create(:user) }
-
     let(:push_data) do
       {
         action: :created,
@@ -251,9 +246,6 @@ describe EventCreateService do
   end
 
   describe 'Project' do
-    let(:user) { create :user }
-    let(:project) { create(:project) }
-
     describe '#join_project' do
       subject { service.join_project(project, user) }
 
@@ -268,4 +260,81 @@ describe EventCreateService do
       it { expect { subject }.to change { Event.count }.from(0).to(1) }
     end
   end
+
+  describe 'design events' do
+    let_it_be(:design) { create(:design, project: project) }
+    let_it_be(:author) { user }
+
+    shared_examples 'feature flag gated multiple event creation' do
+      context 'the feature flag is off' do
+        before do
+          stub_feature_flags(design_activity_events: false)
+        end
+
+        specify { expect(result).to be_empty }
+        specify { expect { result }.not_to change { Event.count } }
+        specify { expect { result }.not_to exceed_query_limit(0) }
+      end
+
+      context 'the feature flag is enabled for a single project' do
+        before do
+          stub_feature_flags(design_activity_events: project)
+        end
+
+        specify { expect(result).not_to be_empty }
+        specify { expect { result }.to change { Event.count }.by(1) }
+      end
+    end
+
+    describe '#save_designs' do
+      let_it_be(:updated) { create_list(:design, 5) }
+      let_it_be(:created) { create_list(:design, 3) }
+
+      let(:result) { service.save_designs(author, create: created, update: updated) }
+
+      specify { expect { result }.to change { Event.count }.by(8) }
+
+      specify { expect { result }.not_to exceed_query_limit(1) }
+
+      it 'creates 3 created design events' do
+        ids = result.pluck('id')
+        events = Event.created_action.where(id: ids)
+
+        expect(events.map(&:design)).to match_array(created)
+      end
+
+      it 'creates 5 created design events' do
+        ids = result.pluck('id')
+        events = Event.updated_action.where(id: ids)
+
+        expect(events.map(&:design)).to match_array(updated)
+      end
+
+      it_behaves_like 'feature flag gated multiple event creation' do
+        let(:project) { created.first.project }
+      end
+    end
+
+    describe '#destroy_designs' do
+      let_it_be(:designs) { create_list(:design, 5) }
+      let_it_be(:author) { create(:user) }
+
+      let(:result) { service.destroy_designs(designs, author) }
+
+      specify { expect { result }.to change { Event.count }.by(5) }
+
+      specify { expect { result }.not_to exceed_query_limit(1) }
+
+      it 'creates 5 destroyed design events' do
+        ids = result.pluck('id')
+        events = Event.destroyed_action.where(id: ids)
+
+        expect(events.map(&:design)).to match_array(designs)
+      end
+
+      it_behaves_like 'feature flag gated multiple event creation' do
+        let(:project) { designs.first.project }
+      end
+    end
+  end
 end