From 48cc651a0d1a645d1aac503b700741b667502ad4 Mon Sep 17 00:00:00 2001
From: Muhsin Keloth <muhsinkeramam@gmail.com>
Date: Mon, 8 Jan 2024 11:29:23 +0530
Subject: [PATCH] fix: review comments

---
 app/finders/notification_finder.rb       |  2 +-
 app/models/notification.rb               |  4 ---
 spec/finders/notification_finder_spec.rb | 41 ++++++++++--------------
 3 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/app/finders/notification_finder.rb b/app/finders/notification_finder.rb
index e4f0ff59e..1408aad2b 100644
--- a/app/finders/notification_finder.rb
+++ b/app/finders/notification_finder.rb
@@ -60,6 +60,6 @@ class NotificationFinder
   end
 
   def notifications
-    @notifications.page(current_page).per(RESULTS_PER_PAGE).order(last_activity_at: :asc)
+    @notifications.page(current_page).per(RESULTS_PER_PAGE).order(last_activity_at: :desc)
   end
 end
diff --git a/app/models/notification.rb b/app/models/notification.rb
index 3cdf8d0be..2ec9a014c 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -47,10 +47,6 @@ class Notification < ApplicationRecord
   after_create_commit :process_notification_delivery, :dispatch_create_event
   after_destroy_commit :dispatch_destroy_event
 
-  # TODO: Get rid of default scope
-  # https://stackoverflow.com/a/1834250/939299
-  default_scope { order(last_activity_at: :desc) }
-
   PRIMARY_ACTORS = ['Conversation'].freeze
 
   def push_event_data
diff --git a/spec/finders/notification_finder_spec.rb b/spec/finders/notification_finder_spec.rb
index 9c07bc13b..7823058ff 100644
--- a/spec/finders/notification_finder_spec.rb
+++ b/spec/finders/notification_finder_spec.rb
@@ -13,50 +13,43 @@ describe NotificationFinder do
     create(:notification, account: account, user: user, updated_at: DateTime.now.utc + 2.days, last_activity_at: DateTime.now.utc + 2.days)
     create(:notification, account: account, user: user, updated_at: DateTime.now.utc + 4.days, last_activity_at: DateTime.now.utc + 4.days,
                           notification_type: :conversation_creation)
+    create(:notification, account: account, user: user, updated_at: DateTime.now.utc + 5.days, last_activity_at: DateTime.now.utc + 5.days,
+                          notification_type: :conversation_mention)
+    create(:notification, account: account, user: user, updated_at: DateTime.now.utc + 6.days, last_activity_at: DateTime.now.utc + 6.days,
+                          notification_type: :participating_conversation_new_message)
   end
 
   describe '#perform' do
-    context 'with snoozed status' do
-      let(:params) { { status: 'snoozed' } }
-
-      it 'filter notifications by status' do
-        result = notification_finder.perform
-        expect(result.length).to be 1
-      end
-    end
-
-    context 'without snoozed status' do
+    context 'when params are empty' do
       let(:params) { {} }
 
-      it 'returns all notifications' do
+      it 'returns notifications which aren not snoozed' do
         result = notification_finder.perform
         expect(result.length).to be 3
       end
-    end
 
-    context 'when order by last_activity_at' do
-      let(:params) { {} }
-
-      it 'returns all notifications' do
+      it 'orders notifications by last activity at' do
         result = notification_finder.perform
         expect(result.first.last_activity_at).to be > result.last.last_activity_at
-        expect(result.first.last_activity_at).to be > result.last.last_activity_at
       end
-    end
 
-    context 'when order by user notification settings' do
-      let(:params) { {} }
-
-      before do
+      it 'filter notifications by user preferences' do
         notification_setting = user.notification_settings.find_by(account_id: account.id)
         notification_setting.selected_email_flags = [:email_conversation_creation]
         notification_setting.save!
-      end
 
-      it 'returns all notifications' do
         result = notification_finder.perform
         expect(result.length).to be 4
       end
     end
+
+    context 'when snoozed param is passed' do
+      let(:params) { { status: 'snoozed' } }
+
+      it 'returns only snoozed notifications' do
+        result = notification_finder.perform
+        expect(result.length).to be 1
+      end
+    end
   end
 end