Merge branch 'ajk-design-activity-a' into 'master'

Add model support for design events

See merge request gitlab-org/gitlab!32700

app/models/event.rb

@@ -22,13 +22,16 @@ class Event < ApplicationRecord
     left:       9, # User left project
     destroyed:  10,
     expired:    11, # User left project due to expiry
-    approved:   12
+    approved:   12,
+    archived:   13 # Recoverable deletion
   ).freeze
 
   private_constant :ACTIONS
 
   WIKI_ACTIONS = [:created, :updated, :destroyed].freeze
 
+  DESIGN_ACTIONS = [:created, :updated, :destroyed, :archived].freeze
+
   TARGET_TYPES = HashWithIndifferentAccess.new(
     issue:          Issue,
     milestone:      Milestone,
@@ -37,7 +40,8 @@ class Event < ApplicationRecord
     project:        Project,
     snippet:        Snippet,
     user:           User,
-    wiki:           WikiPage::Meta
+    wiki:           WikiPage::Meta,
+    design:         DesignManagement::Design
   ).freeze
 
   RESET_PROJECT_ACTIVITY_INTERVAL = 1.hour
@@ -49,6 +53,7 @@ class Event < ApplicationRecord
   delegate :title, to: :issue, prefix: true, allow_nil: true
   delegate :title, to: :merge_request, prefix: true, allow_nil: true
   delegate :title, to: :note, prefix: true, allow_nil: true
+  delegate :title, to: :design, prefix: true, allow_nil: true
 
   belongs_to :author, class_name: "User"
   belongs_to :project
@@ -75,9 +80,11 @@ class Event < ApplicationRecord
   # Scopes
   scope :recent, -> { reorder(id: :desc) }
   scope :for_wiki_page, -> { where(target_type: 'WikiPage::Meta') }
+  scope :for_design, -> { where(target_type: 'DesignManagement::Design') }
 
   # Needed to implement feature flag: can be removed when feature flag is removed
   scope :not_wiki_page, -> { where('target_type IS NULL or target_type <> ?', 'WikiPage::Meta') }
+  scope :not_design, -> { where('target_type IS NULL or target_type <> ?', 'DesignManagement::Design') }
 
   scope :with_associations, -> do
     # We're using preload for "push_event_payload" as otherwise the association
@@ -96,6 +103,13 @@ class Event < ApplicationRecord
   # should ensure the ID points to a valid user.
   validates :author_id, presence: true
 
+  validates :action_enum_value,
+    if: :design?,
+    inclusion: {
+      in: actions.values_at(*DESIGN_ACTIONS),
+      message: ->(event, _data) { "#{event.action} is not a valid design action" }
+    }
+
   self.inheritance_column = 'action'
 
   class << self
@@ -190,6 +204,10 @@ class Event < ApplicationRecord
     target_type == 'WikiPage::Meta'
   end
 
+  def design?
+    target_type == 'DesignManagement::Design'
+  end
+
   def milestone
     target if milestone?
   end
@@ -198,6 +216,10 @@ class Event < ApplicationRecord
     target if issue?
   end
 
+  def design
+    target if design?
+  end
+
   def merge_request
     target if merge_request?
   end
@@ -217,6 +239,8 @@ class Event < ApplicationRecord
   def action_name
     if push_action?
       push_action_name
+    elsif design?
+      design_action_names[action.to_sym]
     elsif closed_action?
       "closed"
     elsif merged_action?
@@ -358,7 +382,7 @@ class Event < ApplicationRecord
                         :read_milestone
                       elsif wiki_page?
                         :read_wiki
-                      elsif design_note?
+                      elsif design_note? || design?
                         :read_design
                       end
                     end
@@ -412,6 +436,19 @@ class Event < ApplicationRecord
     # (because the table does not exist yet).
     UserInteractedProject.track(self) if UserInteractedProject.available?
   end
+
+  def design_action_names
+    {
+      created: _('uploaded'),
+      updated: _('revised'),
+      destroyed: _('deleted'),
+      archived: _('archived')
+    }
+  end
+
+  def action_enum_value
+    self.class.actions[action]
+  end
 end
 
 Event.prepend_if_ee('EE::Event')

locale/gitlab.pot

@@ -26901,6 +26901,9 @@ msgstr ""
 msgid "resolved the corresponding error and closed the issue."
 msgstr ""
 
+msgid "revised"
+msgstr ""
+
 msgid "score"
 msgstr ""
 
@@ -27033,6 +27036,9 @@ msgstr ""
 msgid "updated %{time_ago}"
 msgstr ""
 
+msgid "uploaded"
+msgstr ""
+
 msgid "uploads"
 msgstr ""
 

spec/factories/events.rb

@@ -17,6 +17,7 @@ FactoryBot.define do
     trait(:left)      { action { :left } }
     trait(:destroyed) { action { :destroyed } }
     trait(:expired)   { action { :expired } }
+    trait(:archived)  { action { :archived } }
 
     factory :closed_issue_event do
       action { :closed }
@@ -33,15 +34,27 @@ FactoryBot.define do
       end
     end
 
-    trait :for_design do
+    trait :has_design do
       transient do
         design { create(:design, issue: create(:issue, project: project)) }
+      end
+    end
+
+    trait :for_design do
+      has_design
+
+      transient do
         note { create(:note, author: author, project: project, noteable: design) }
       end
 
       action { :commented }
       target { note }
     end
+
+    factory :design_event, traits: [:has_design] do
+      action { :created }
+      target { design }
+    end
   end
 
   factory :push_event, class: 'PushEvent' do

spec/models/event_spec.rb

@@ -13,6 +13,7 @@ describe Event do
     it { is_expected.to respond_to(:author_email) }
     it { is_expected.to respond_to(:issue_title) }
     it { is_expected.to respond_to(:merge_request_title) }
+    it { is_expected.to respond_to(:design_title) }
   end
 
   describe 'Callbacks' do
@@ -84,6 +85,26 @@ describe Event do
     end
   end
 
+  describe 'validations' do
+    describe 'action' do
+      context 'for a design' do
+        where(:action, :valid) do
+          valid = described_class::DESIGN_ACTIONS.map(&:to_s).to_set
+
+          described_class.actions.keys.map do |action|
+            [action, valid.include?(action)]
+          end
+        end
+
+        with_them do
+          let(:event) { build(:design_event, action: action) }
+
+          specify { expect(event.valid?).to eq(valid) }
+        end
+      end
+    end
+  end
+
   describe 'scopes' do
     describe 'created_at' do
       it 'can find the right event' do
@@ -552,7 +573,7 @@ describe Event do
       end
     end
 
-    context 'design event' do
+    context 'design note event' do
       include DesignManagementTestHelpers
 
       let(:target) { note_on_design }
@@ -577,6 +598,32 @@ describe Event do
         include_examples 'visible to assignee and author', true
       end
     end
+
+    context 'design event' do
+      include DesignManagementTestHelpers
+
+      let(:target) { design }
+
+      before do
+        enable_design_management
+      end
+
+      include_examples 'visibility examples' do
+        let(:visibility) { visible_to_all }
+      end
+
+      include_examples 'visible to assignee and author', true
+
+      context 'the event refers to a design on a confidential issue' do
+        let(:design) { create(:design, issue: confidential_issue, project: project) }
+
+        include_examples 'visibility examples' do
+          let(:visibility) { visible_to_none_except(:member, :admin) }
+        end
+
+        include_examples 'visible to assignee and author', true
+      end
+    end
   end
 
   describe 'wiki_page predicate scopes' do
@@ -587,10 +634,31 @@ describe Event do
         create(:wiki_page_event),
         create(:closed_issue_event),
         create(:event, :created),
-        create(:wiki_page_event)
+        create(:design_event, :destroyed),
+        create(:wiki_page_event),
+        create(:design_event)
       ]
     end
 
+    describe '.for_design' do
+      it 'only includes design events' do
+        design_events = events.select(&:design?)
+
+        expect(described_class.for_design)
+          .to be_present
+          .and match_array(design_events)
+      end
+    end
+
+    describe '.not_design' do
+      it 'does not contain the design events' do
+        non_design_events = events.reject(&:design?)
+
+        expect(events).not_to match_array(non_design_events)
+        expect(described_class.not_design).to match_array(non_design_events)
+      end
+    end
+
     describe '.for_wiki_page' do
       it 'only contains the wiki page events' do
         wiki_events = events.select(&:wiki_page?)
@@ -618,9 +686,39 @@ describe Event do
     end
   end
 
-  describe '#wiki_page and #wiki_page?' do
+  describe 'categorization' do
     let_it_be(:project) { create(:project, :repository) }
+    let_it_be(:all_valid_events) do
+      # mapping from factory name to whether we need to supply the project
+      valid_target_factories = {
+        issue: true,
+        note_on_issue: true,
+        user: false,
+        merge_request: true,
+        note_on_merge_request: true,
+        project_snippet: true,
+        personal_snippet: false,
+        note_on_project_snippet: true,
+        note_on_personal_snippet: false,
+        wiki_page_meta: true,
+        milestone: true,
+        project: false,
+        design: true,
+        note_on_design: true,
+        note_on_commit: true
+      }
+      valid_target_factories.map do |kind, needs_project|
+        extra_data = needs_project ? { project: project } : {}
+        target = kind == :project ? nil : build(kind, **extra_data)
+        [kind, build(:event, :created, project: project, target: target)]
+      end.to_h
+    end
+
+    it 'passes a sanity check', :aggregate_failures do
+      expect(all_valid_events.values).to all(be_valid)
+    end
 
+    describe '#wiki_page and #wiki_page?' do
       context 'for a wiki page event' do
         let(:wiki_page) do
           create(:wiki_page, project: project)
@@ -631,11 +729,10 @@ describe Event do
         it { is_expected.to have_attributes(wiki_page?: be_truthy, wiki_page: wiki_page) }
       end
 
-    [:issue, :user, :merge_request, :snippet, :milestone, nil].each do |kind|
-      context "for a #{kind} event" do
-        it 'is nil' do
-          target = create(kind) if kind
-          event = create(:event, project: project, target: target)
+      context 'for any other event' do
+        it 'has no wiki_page and is not a wiki_page', :aggregate_failures do
+          all_valid_events.each do |k, event|
+            next if k == :wiki_page_meta
 
             expect(event).to have_attributes(wiki_page: be_nil, wiki_page?: be_falsy)
           end
@@ -643,6 +740,27 @@ describe Event do
       end
     end
 
+    describe '#design and #design?' do
+      context 'for a design event' do
+        let(:design) { build(:design, project: project) }
+
+        subject(:event) { build(:design_event, target: design, project: project) }
+
+        it { is_expected.to have_attributes(design?: be_truthy, design: design) }
+      end
+
+      context 'for any other event' do
+        it 'has no design and is not a design', :aggregate_failures do
+          all_valid_events.each do |k, event|
+            next if k == :design
+
+            expect(event).to have_attributes(design: be_nil, design?: be_falsy)
+          end
+        end
+      end
+    end
+  end
+
   describe '.limit_recent' do
     let!(:event1) { create(:closed_issue_event) }
     let!(:event2) { create(:closed_issue_event) }
@@ -765,6 +883,19 @@ describe Event do
     end
   end
 
+  describe '#action_name' do
+    it 'handles all valid design events' do
+      created, updated, destroyed, archived = %i[created updated destroyed archived].map do |trait|
+        build(:design_event, trait).action_name
+      end
+
+      expect(created).to eq('uploaded')
+      expect(updated).to eq('revised')
+      expect(destroyed).to eq('deleted')
+      expect(archived).to eq('archived')
+    end
+  end
+
   def create_push_event(project, user)
     event = create(:push_event, project: project, author: user)