DEV: Remove remaining hardcoded ids (#18735)

spec/fabricators/user_fabricator.rb

@@ -132,3 +132,10 @@ end
 Fabricator(:unicode_user, from: :user) do
   username { sequence(:username) { |i| "Löwe#{i}" } }
 end
+
+Fabricator(:bot, from: :user) do
+  id do
+    min_id = User.minimum(:id)
+    [(min_id || 0) - 1, -10].min
+  end
+end

spec/lib/email/processor_spec.rb

@@ -172,14 +172,13 @@ RSpec.describe Email::Processor do
   end
 
   describe 'when replying to a post that is too old' do
-    let(:mail) { file_from_fixtures("old_destination.eml", "emails").read }
     fab!(:user) { Fabricate(:user, email: "discourse@bar.com") }
+    fab!(:topic) { Fabricate(:topic) }
+    fab!(:post) { Fabricate(:post, topic: topic, created_at: 3.days.ago) }
+    let(:mail) { file_from_fixtures("old_destination.eml", "emails").read.gsub("424242", topic.id.to_s).gsub("123456", post.id.to_s) }
+
     it 'rejects the email with the right response' do
       SiteSetting.disallow_reply_by_email_after_days = 2
-
-      topic = Fabricate(:topic, id: 424242)
-      post  = Fabricate(:post, topic: topic, id: 123456, created_at: 3.days.ago)
-
       processor = Email::Processor.new(mail)
       processor.process!
 

spec/lib/email/receiver_spec.rb

@@ -83,18 +83,19 @@ RSpec.describe Email::Receiver do
   it "raises an OldDestinationError when notification is too old" do
     SiteSetting.disallow_reply_by_email_after_days = 2
 
-    topic = Fabricate(:topic, id: 424242)
-    post  = Fabricate(:post, topic: topic, id: 123456)
+    topic = Fabricate(:topic)
+    post  = Fabricate(:post, topic: topic)
     user  = Fabricate(:user, email: "discourse@bar.com")
 
-    expect { process(:old_destination) }.to raise_error(
+    mail = email(:old_destination).gsub("424242", topic.id.to_s)
+    expect { Email::Receiver.new(mail).process! }.to raise_error(
       Email::Receiver::BadDestinationAddress
     )
 
     IncomingEmail.destroy_all
     post.update!(created_at: 3.days.ago)
 
-    expect { process(:old_destination) }.to raise_error(
+    expect { Email::Receiver.new(mail).process! }.to raise_error(
       Email::Receiver::OldDestinationError
     )
     expect(IncomingEmail.last.error).to eq("Email::Receiver::OldDestinationError")
@@ -102,7 +103,7 @@ RSpec.describe Email::Receiver do
     SiteSetting.disallow_reply_by_email_after_days = 0
     IncomingEmail.destroy_all
 
-    expect { process(:old_destination) }.to raise_error(
+    expect { Email::Receiver.new(mail).process! }.to raise_error(
       Email::Receiver::BadDestinationAddress
     )
   end
@@ -434,7 +435,7 @@ RSpec.describe Email::Receiver do
       expect(topic.posts.last.raw).to eq("This will not include the previous discussion that is present in this email.")
     end
 
-    it "removes the trnaslated 'Previous Replies' marker" do
+    it "removes the translated 'Previous Replies' marker" do
       expect { process(:previous_replies_de) }.to change { topic.posts.count }
       expect(topic.posts.last.raw).to eq("This will not include the previous discussion that is present in this email.")
     end

spec/lib/file_store/base_store_spec.rb

@@ -1,7 +1,10 @@
 # frozen_string_literal: true
 
 RSpec.describe FileStore::BaseStore do
-  fab!(:upload) { Fabricate(:upload, id: 9999, sha1: Digest::SHA1.hexdigest('9999')) }
+  fab!(:upload) do
+    Upload.delete(9999) # In case of any collisions
+    Fabricate(:upload, id: 9999, sha1: Digest::SHA1.hexdigest('9999'))
+  end
 
   describe '#get_path_for_upload' do
     def expect_correct_path(expected_path)

spec/lib/guardian_spec.rb

@@ -303,7 +303,7 @@ RSpec.describe Guardian do
       end
 
       it "returns true for bot user" do
-        expect(Guardian.new(Fabricate(:user, id: -19876)).can_send_private_message?(another_user)).to be_truthy
+        expect(Guardian.new(Fabricate(:bot)).can_send_private_message?(another_user)).to be_truthy
       end
 
       it "returns true for system user" do

spec/lib/site_setting_extension_spec.rb

@@ -771,7 +771,8 @@ RSpec.describe SiteSettingExtension do
   describe '.all_settings' do
     describe 'uploads settings' do
       it 'should return the right values' do
-        system_upload = Fabricate(:upload, id: -999)
+        negative_upload_id = [(Upload.minimum(:id) || 0) - 1, -10].min
+        system_upload = Fabricate(:upload, id: negative_upload_id)
         settings.setting(:logo, system_upload.id, type: :upload)
         settings.refresh!
         setting = settings.all_settings.last

spec/lib/unread_spec.rb

@@ -3,22 +3,22 @@
 require 'unread'
 
 RSpec.describe Unread do
-
   let(:whisperers_group) { Fabricate(:group) }
-  let(:user) { Fabricate(:user, id: 1, groups: [whisperers_group]) }
+  let(:user) { Fabricate(:user, groups: [whisperers_group]) }
   let(:topic) do
-    Fabricate.build(:topic,
-                       posts_count: 13,
-                       highest_staff_post_number: 15,
-                       highest_post_number: 13,
-                       id: 1)
+    Fabricate(:topic,
+      posts_count: 13,
+      highest_staff_post_number: 15,
+      highest_post_number: 13,
+    )
   end
 
-  let (:topic_user) do
-    Fabricate.build(:topic_user,
-                        notification_level: TopicUser.notification_levels[:tracking],
-                        topic_id: topic.id,
-                        user_id: user.id)
+  let(:topic_user) do
+    Fabricate(:topic_user,
+      notification_level: TopicUser.notification_levels[:tracking],
+      topic_id: topic.id,
+      user_id: user.id
+    )
   end
 
   def unread
@@ -73,10 +73,9 @@ RSpec.describe Unread do
       expect(unread.unread_posts).to eq(0)
     end
 
-    it 'has 0 unread psots if the user has not seen the topic' do
+    it 'has 0 unread posts if the user has not seen the topic' do
       topic_user.last_read_post_number = nil
       expect(unread.unread_posts).to eq(0)
     end
   end
-
 end

spec/lib/validators/post_validator_spec.rb

@@ -23,7 +23,7 @@ RSpec.describe PostValidator do
     end
 
     context "when post's topic is a PM between a human and a non human user" do
-      fab!(:robot) { Fabricate(:user, id: -3) }
+      fab!(:robot) { Fabricate(:bot) }
       fab!(:user) { Fabricate(:user) }
 
       let(:topic) do

spec/models/topic_spec.rb

@@ -2801,7 +2801,7 @@ RSpec.describe Topic do
   end
 
   describe '#pm_with_non_human_user?' do
-    fab!(:robot) { Fabricate(:user, id: -3) }
+    fab!(:robot) { Fabricate(:bot) }
 
     fab!(:topic) do
       topic = Fabricate(:private_message_topic,

spec/models/user_spec.rb

@@ -1992,7 +1992,7 @@ RSpec.describe User do
 
   describe '.human_users' do
     it 'should only return users with a positive primary key' do
-      Fabricate(:user, id: -1979)
+      Fabricate(:bot)
       user = Fabricate(:user)
 
       expect(User.human_users).to eq([user])

spec/requests/groups_controller_spec.rb

@@ -1136,8 +1136,7 @@ RSpec.describe GroupsController do
       )
     end
 
-    fab!(:bot) { Fabricate(:user, id: -999) }
-
+    fab!(:bot) { Fabricate(:bot) }
     let(:group) { Fabricate(:group, users: [user1, user2, user3, bot]) }
 
     it "should allow members to be sorted by" do

spec/script/import_scripts/vanilla_body_parser_spec.rb

@@ -8,8 +8,8 @@ RSpec.describe VanillaBodyParser do
   let(:lookup) { ImportScripts::LookupContainer.new }
   let(:uploader) { ImportScripts::Uploader.new }
   let(:uploads_path) { 'spec/fixtures/images/vanilla_import' }
-  let(:user) { Fabricate(:user, id: '34567', email: 'saruman@maiar.org', name: 'Saruman, Multicolor', username: 'saruman_multicolor') }
-  let(:user_id) { lookup.add_user('34567', user) }
+  let(:user) { Fabricate(:user, email: 'saruman@maiar.org', name: 'Saruman, Multicolor', username: 'saruman_multicolor') }
+  let(:user_id) { lookup.add_user(user.id.to_s, user) }
 
   before do
     STDOUT.stubs(:write)
@@ -68,10 +68,11 @@ this starts with spaces but IS NOT a quote'''
     end
 
     it 'supports mentions imported users' do
-      mentioned = Fabricate(:user, id: '666', email: 'gandalf@maiar.com', name: 'Gandalf The Grey', username: 'gandalf_the_grey')
-      lookup.add_user('666', mentioned)
+      mentioned = Fabricate(:user, email: 'gandalf@maiar.com', name: 'Gandalf The Grey', username: 'gandalf_the_grey')
+      lookup.add_user(mentioned.id.to_s, mentioned)
 
-      parsed = VanillaBodyParser.new({ 'Format' => 'Rich', 'Body' => rich_bodies[:mention].to_json }, user_id).parse
+      body = rich_bodies[:mention].to_json.gsub("666", mentioned.id.to_s)
+      parsed = VanillaBodyParser.new({ 'Format' => 'Rich', 'Body' => body }, user_id).parse
       expect(parsed).to eq "@gandalf_the_grey, what do you think?"
     end
 
@@ -91,7 +92,8 @@ this starts with spaces but IS NOT a quote'''
       topic_id = lookup.add_topic(post)
       lookup.add_post('discussion#12345', post)
 
-      parsed = VanillaBodyParser.new({ 'Format' => 'Rich', 'Body' => rich_bodies[:quote].to_json }, user_id).parse
+      body = rich_bodies[:quote].to_json.gsub("34567", user.id.to_s)
+      parsed = VanillaBodyParser.new({ 'Format' => 'Rich', 'Body' => body }, user_id).parse
       expect(parsed).to eq "[quote=\"#{user.username}, post: #{post.post_number}, topic: #{post.topic.id}\"]\n\nThis is the full<br \/>body<br \/>of the quoted discussion.<br \/>\n\n[/quote]\n\nWhen did this happen?"
     end
 

spec/serializers/current_user_serializer_spec.rb

@@ -297,7 +297,7 @@ RSpec.describe CurrentUserSerializer do
     let(:category_sidebar_section_link) { Fabricate(:category_sidebar_section_link, user: user) }
     let(:category_sidebar_section_link_2) { Fabricate(:category_sidebar_section_link, user: user) }
 
-    it "is not included when SiteSeting.enable_experimental_sidebar_hamburger is false" do
+    it "is not included when SiteSetting.enable_experimental_sidebar_hamburger is false" do
       category_sidebar_section_link
       SiteSetting.enable_experimental_sidebar_hamburger = false
 

spec/serializers/user_card_serializer_spec.rb

@@ -2,7 +2,7 @@
 
 RSpec.describe UserCardSerializer do
   context "with a TL0 user seen as anonymous" do
-    let(:user) { Fabricate.build(:user, trust_level: 0, user_profile: Fabricate.build(:user_profile)) }
+    let(:user) { Fabricate(:user, trust_level: 0) }
     let(:serializer) { described_class.new(user, scope: Guardian.new, root: false) }
     let(:json) { serializer.as_json }
 
@@ -14,12 +14,9 @@ RSpec.describe UserCardSerializer do
 
   context "as current user" do
     it "serializes emails correctly" do
-      user = Fabricate.build(:user,
-                             id: 1,
-                             user_profile: Fabricate.build(:user_profile),
-                             user_option: UserOption.new(dynamic_favicon: true),
-                             user_stat: UserStat.new
-                            )
+      user = Fabricate(:user)
+      user.user_option.update(dynamic_favicon: true)
+
       json = described_class.new(user, scope: Guardian.new(user), root: false).as_json
       expect(json[:secondary_emails]).to eq([])
       expect(json[:unconfirmed_emails]).to eq([])
@@ -29,6 +26,7 @@ RSpec.describe UserCardSerializer do
   context "as different user" do
     let(:user) { Fabricate(:user, trust_level: 0) }
     let(:user2) { Fabricate(:user, trust_level: 1) }
+
     it "does not serialize emails" do
       json = described_class.new(user, scope: Guardian.new(user2), root: false).as_json
       expect(json[:secondary_emails]).to be_nil

spec/serializers/user_serializer_spec.rb

@@ -38,13 +38,8 @@ RSpec.describe UserSerializer do
       SiteSetting.default_other_notification_level_when_replying = 3
       SiteSetting.default_other_new_topic_duration_minutes = 60 * 24
 
-      user = Fabricate.build(:user,
-                             id: 1,
-                             user_profile: Fabricate.build(:user_profile),
-                             user_option: UserOption.new(dynamic_favicon: true, skip_new_user_tips: true),
-                             user_stat: UserStat.new,
-                             created_at: Time.zone.now
-                            )
+      user = Fabricate(:user)
+      user.user_option.update(dynamic_favicon: true, skip_new_user_tips: true)
 
       json = UserSerializer.new(user, scope: Guardian.new(user), root: false).as_json
 
@@ -384,14 +379,14 @@ RSpec.describe UserSerializer do
     context 'when viewing self' do
       subject(:json) { UserSerializer.new(user, scope: Guardian.new(user), root: false).as_json }
 
-      it "is not included when SiteSeting.enable_experimental_sidebar_hamburger is false" do
+      it "is not included when SiteSetting.enable_experimental_sidebar_hamburger is false" do
         SiteSetting.enable_experimental_sidebar_hamburger = false
         SiteSetting.tagging_enabled = true
 
         expect(json[:sidebar_tags]).to eq(nil)
       end
 
-      it "is not included when SiteSeting.tagging_enabled is false" do
+      it "is not included when SiteSetting.tagging_enabled is false" do
         SiteSetting.enable_experimental_sidebar_hamburger = true
         SiteSetting.tagging_enabled = false