Fix issues with optional merge requests approval in CE

Fix note about MRs approves in activity
Fix icons on approve note in MR feed

app/helpers/system_note_helper.rb

@@ -2,6 +2,8 @@
 
 module SystemNoteHelper
   ICON_NAMES_BY_ACTION = {
+    'approved' => 'approval',
+    'unapproved' => 'unapproval',
     'cherry_pick' => 'cherry-pick-commit',
     'commit' => 'commit',
     'description' => 'pencil-square',

app/models/event.rb

@@ -242,6 +242,8 @@ class Event < ApplicationRecord
     target if note?
   end
 
+  # rubocop: disable Metrics/CyclomaticComplexity
+  # rubocop: disable Metrics/PerceivedComplexity
   def action_name
     if push_action?
       push_action_name
@@ -267,10 +269,14 @@ class Event < ApplicationRecord
       'updated'
     elsif created_project_action?
       created_project_action_name
+    elsif approved_action?
+      'approved'
     else
       "opened"
     end
   end
+  # rubocop: enable Metrics/CyclomaticComplexity
+  # rubocop: enable Metrics/PerceivedComplexity
 
   def target_iid
     target.respond_to?(:iid) ? target.iid : target_id

changelogs/unreleased/231306-approve-activity-in-core.yml

+---
+title: Fix issues with optional merge requests approval in CE
+author: Pavel Kuznetsov
+merge_request: 42119
+type: fixed

ee/app/helpers/ee/system_note_helper.rb

@@ -5,8 +5,6 @@ module EE
     extend ::Gitlab::Utils::Override
 
     EE_ICON_NAMES_BY_ACTION = {
-      'approved' => 'approval',
-      'unapproved' => 'unapproval',
       'relate' => 'link',
       'unrelate' => 'unlink',
       'epic_issue_added' => 'issues',

ee/app/models/ee/event.rb

@@ -18,15 +18,6 @@ module EE
       super.merge(read_epic: %i[epic? epic_note?])
     end
 
-    override :action_name
-    def action_name
-      if approved_action?
-        'approved'
-      else
-        super
-      end
-    end
-
     def epic_note?
       note? && note_target.is_a?(::Epic)
     end

ee/spec/factories/events.rb

@@ -9,7 +9,5 @@ FactoryBot.modify do
       action { :created }
       project { nil }
     end
-
-    trait(:approved) { action { :approved } }
   end
 end

ee/spec/models/ee/event_spec.rb

@@ -121,24 +121,4 @@ RSpec.describe Event do
       end
     end
   end
-
-  describe '#action_name' do
-    let_it_be(:approved_event) {create(:event, :approved)}
-    let_it_be(:created_event) {create(:event, :created)}
-
-    it 'returns the appropriate action name' do
-      expect(approved_event.action_name).to eq 'approved'
-      expect(created_event.action_name).to eq 'created'
-    end
-  end
-
-  describe '#approved_action?' do
-    let_it_be(:approved_event) {create(:event, :approved)}
-    let_it_be(:created_event) {create(:event, :created)}
-
-    it 'return true only for approved event type' do
-      expect(approved_event.approved_action?).to be true
-      expect(created_event.approved_action?).to be false
-    end
-  end
 end

spec/factories/events.rb

@@ -18,6 +18,7 @@ FactoryBot.define do
     trait(:destroyed) { action { :destroyed } }
     trait(:expired)   { action { :expired } }
     trait(:archived)  { action { :archived } }
+    trait(:approved)  { action { :approved } }
 
     factory :closed_issue_event do
       action { :closed }
@@ -55,6 +56,16 @@ FactoryBot.define do
       action { :created }
       target { design }
     end
+
+    factory :project_created_event do
+      project factory: :project
+      action { :created }
+    end
+
+    factory :project_imported_event do
+      project factory: [:project, :with_import_url]
+      action { :created }
+    end
   end
 
   factory :push_event, class: 'PushEvent' do

spec/factories/projects.rb

@@ -283,6 +283,12 @@ FactoryBot.define do
       end
     end
 
+    trait :with_import_url do
+      import_finished
+
+      import_url { generate(:url) }
+    end
+
     trait(:wiki_enabled)            { wiki_access_level { ProjectFeature::ENABLED } }
     trait(:wiki_disabled)           { wiki_access_level { ProjectFeature::DISABLED } }
     trait(:wiki_private)            { wiki_access_level { ProjectFeature::PRIVATE } }

spec/models/event_spec.rb

@@ -918,6 +918,56 @@ RSpec.describe Event do
       expect(destroyed).to eq('deleted')
       expect(archived).to eq('archived')
     end
+
+    it 'handles correct push_action' do
+      project = create(:project)
+      user = create(:user)
+      project.add_developer(user)
+      push_event = create_push_event(project, user)
+
+      expect(push_event.push_action?).to be true
+      expect(push_event.action_name).to eq('pushed to')
+    end
+
+    context 'handles correct base actions' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:trait, :action_name) do
+        :created   | 'created'
+        :updated   | 'opened'
+        :closed    | 'closed'
+        :reopened  | 'opened'
+        :commented | 'commented on'
+        :merged    | 'accepted'
+        :joined    | 'joined'
+        :left      | 'left'
+        :destroyed | 'destroyed'
+        :expired   | 'removed due to membership expiration from'
+        :approved  | 'approved'
+      end
+
+      with_them do
+        it 'with correct name and method' do
+          event = build(:event, trait)
+
+          expect(event.action_name).to eq(action_name)
+        end
+      end
+    end
+
+    context 'for created_project_action?' do
+      it 'returns created for created event' do
+        action = build(:project_created_event)
+
+        expect(action.action_name).to eq('created')
+      end
+
+      it 'returns imported for imported event' do
+        action = build(:project_imported_event)
+
+        expect(action.action_name).to eq('imported')
+      end
+    end
   end
 
   def create_push_event(project, user)