From 7db9e9fa8343c773fe72cffe73a2d0ae412e22b8 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Mon, 3 Jun 2024 13:39:17 -0400
Subject: [PATCH] Improve data setup naming in grouped notification specs

---
 spec/models/notification_spec.rb | 91 ++++++++++++++++----------------
 1 file changed, 45 insertions(+), 46 deletions(-)

diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb
index d498ee02a55..f55c7c343cd 100644
--- a/spec/models/notification_spec.rb
+++ b/spec/models/notification_spec.rb
@@ -151,62 +151,61 @@ RSpec.describe Notification do
     end
   end
 
-  describe '.paginate_groups_by_max_id' do
+  context 'with grouped notifications' do
     let(:account) { Fabricate(:account) }
 
-    let!(:notifications) do
-      ['group-1', 'group-1', nil, 'group-2', nil, 'group-1', 'group-2', 'group-1']
-        .map { |group_key| Fabricate(:notification, account: account, group_key: group_key) }
-    end
+    let!(:group_one_oldest) { Fabricate(:notification, account: account, group_key: 'group-1') }
+    let!(:group_one_old) { Fabricate(:notification, account: account, group_key: 'group-1') }
+    let!(:group_nil_old) { Fabricate(:notification, account: account, group_key: nil) }
+    let!(:group_two_old) { Fabricate(:notification, account: account, group_key: 'group-2') }
+    let!(:group_nil_new) { Fabricate(:notification, account: account, group_key: nil) }
+    let!(:group_one_new) { Fabricate(:notification, account: account, group_key: 'group-1') }
+    let!(:group_two_new) { Fabricate(:notification, account: account, group_key: 'group-2') }
+    let!(:group_one_newest) { Fabricate(:notification, account: account, group_key: 'group-1') }
 
-    context 'without since_id or max_id' do
-      it 'returns the most recent notifications, only keeping one notification per group' do
-        expect(described_class.without_suspended.paginate_groups_by_max_id(4).pluck(:id))
-          .to eq [notifications[7], notifications[6], notifications[4], notifications[2]].pluck(:id)
+    describe '.paginate_groups_by_max_id' do
+      context 'without since_id or max_id' do
+        it 'returns the most recent notifications, only keeping one notification per group' do
+          expect(described_class.without_suspended.paginate_groups_by_max_id(4))
+            .to eq [group_one_newest, group_two_new, group_nil_new, group_nil_old]
+        end
+      end
+
+      context 'with since_id' do
+        it 'returns the most recent notifications, only keeping one notification per group' do
+          expect(described_class.without_suspended.paginate_groups_by_max_id(4, since_id: group_nil_new.id))
+            .to eq [group_one_newest, group_two_new]
+        end
+      end
+
+      context 'with max_id' do
+        it 'returns the most recent notifications after max_id, only keeping one notification per group' do
+          expect(described_class.without_suspended.paginate_groups_by_max_id(4, max_id: group_one_newest.id))
+            .to eq [group_two_new, group_one_new, group_nil_new, group_nil_old]
+        end
       end
     end
 
-    context 'with since_id' do
-      it 'returns the most recent notifications, only keeping one notification per group' do
-        expect(described_class.without_suspended.paginate_groups_by_max_id(4, since_id: notifications[4].id).pluck(:id))
-          .to eq [notifications[7], notifications[6]].pluck(:id)
+    describe '.paginate_groups_by_min_id' do
+      context 'without min_id or max_id' do
+        it 'returns the oldest notifications, only keeping one notification per group' do
+          expect(described_class.without_suspended.paginate_groups_by_min_id(4))
+            .to eq [group_one_oldest, group_nil_old, group_two_old, group_nil_new]
+        end
       end
-    end
 
-    context 'with max_id' do
-      it 'returns the most recent notifications after max_id, only keeping one notification per group' do
-        expect(described_class.without_suspended.paginate_groups_by_max_id(4, max_id: notifications[7].id).pluck(:id))
-          .to eq [notifications[6], notifications[5], notifications[4], notifications[2]].pluck(:id)
+      context 'with max_id' do
+        it 'returns the oldest notifications, stopping at max_id, only keeping one notification per group' do
+          expect(described_class.without_suspended.paginate_groups_by_min_id(4, max_id: group_nil_new.id))
+            .to eq [group_one_oldest, group_nil_old, group_two_old]
+        end
       end
-    end
-  end
 
-  describe '.paginate_groups_by_min_id' do
-    let(:account) { Fabricate(:account) }
-
-    let!(:notifications) do
-      ['group-1', 'group-1', nil, 'group-2', nil, 'group-1', 'group-2', 'group-1']
-        .map { |group_key| Fabricate(:notification, account: account, group_key: group_key) }
-    end
-
-    context 'without min_id or max_id' do
-      it 'returns the oldest notifications, only keeping one notification per group' do
-        expect(described_class.without_suspended.paginate_groups_by_min_id(4).pluck(:id))
-          .to eq [notifications[0], notifications[2], notifications[3], notifications[4]].pluck(:id)
-      end
-    end
-
-    context 'with max_id' do
-      it 'returns the oldest notifications, stopping at max_id, only keeping one notification per group' do
-        expect(described_class.without_suspended.paginate_groups_by_min_id(4, max_id: notifications[4].id).pluck(:id))
-          .to eq [notifications[0], notifications[2], notifications[3]].pluck(:id)
-      end
-    end
-
-    context 'with min_id' do
-      it 'returns the most oldest notifications after min_id, only keeping one notification per group' do
-        expect(described_class.without_suspended.paginate_groups_by_min_id(4, min_id: notifications[0].id).pluck(:id))
-          .to eq [notifications[1], notifications[2], notifications[3], notifications[4]].pluck(:id)
+      context 'with min_id' do
+        it 'returns the most oldest notifications after min_id, only keeping one notification per group' do
+          expect(described_class.without_suspended.paginate_groups_by_min_id(4, min_id: group_one_oldest.id))
+            .to eq [group_one_old, group_nil_old, group_two_old, group_nil_new]
+        end
       end
     end
   end