From df56ab172ae94ba0d9df436c06b77064d83b730e Mon Sep 17 00:00:00 2001 From: Jarek Radosz Date: Tue, 25 Oct 2022 09:29:09 +0200 Subject: [PATCH] DEV: Remove remaining hardcoded ids (#18735) --- spec/fabricators/user_fabricator.rb | 7 +++++ spec/lib/email/processor_spec.rb | 9 +++---- spec/lib/email/receiver_spec.rb | 13 ++++----- spec/lib/file_store/base_store_spec.rb | 5 +++- spec/lib/guardian_spec.rb | 2 +- spec/lib/site_setting_extension_spec.rb | 3 ++- spec/lib/unread_spec.rb | 27 +++++++++---------- spec/lib/validators/post_validator_spec.rb | 2 +- spec/models/topic_spec.rb | 2 +- spec/models/user_spec.rb | 2 +- spec/requests/groups_controller_spec.rb | 3 +-- .../vanilla_body_parser_spec.rb | 14 +++++----- .../current_user_serializer_spec.rb | 2 +- spec/serializers/user_card_serializer_spec.rb | 12 ++++----- spec/serializers/user_serializer_spec.rb | 13 +++------ 15 files changed, 60 insertions(+), 56 deletions(-) diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index 385340ea08f..09ad6c57cc1 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/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 diff --git a/spec/lib/email/processor_spec.rb b/spec/lib/email/processor_spec.rb index 01a0dc61612..42b6efe37e7 100644 --- a/spec/lib/email/processor_spec.rb +++ b/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! diff --git a/spec/lib/email/receiver_spec.rb b/spec/lib/email/receiver_spec.rb index f286e2b334f..fe906377420 100644 --- a/spec/lib/email/receiver_spec.rb +++ b/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 diff --git a/spec/lib/file_store/base_store_spec.rb b/spec/lib/file_store/base_store_spec.rb index cd45506413d..18bf38d08ce 100644 --- a/spec/lib/file_store/base_store_spec.rb +++ b/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) diff --git a/spec/lib/guardian_spec.rb b/spec/lib/guardian_spec.rb index 59a219f73d7..f9f3bfe2348 100644 --- a/spec/lib/guardian_spec.rb +++ b/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 diff --git a/spec/lib/site_setting_extension_spec.rb b/spec/lib/site_setting_extension_spec.rb index ad39c656ef5..bf5fd92b389 100644 --- a/spec/lib/site_setting_extension_spec.rb +++ b/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 diff --git a/spec/lib/unread_spec.rb b/spec/lib/unread_spec.rb index 8b953fbcaf6..88d3a1c66a1 100644 --- a/spec/lib/unread_spec.rb +++ b/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 diff --git a/spec/lib/validators/post_validator_spec.rb b/spec/lib/validators/post_validator_spec.rb index b167130e9a5..79255723bd5 100644 --- a/spec/lib/validators/post_validator_spec.rb +++ b/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 diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 3b569b901e6..e98d872f413 100644 --- a/spec/models/topic_spec.rb +++ b/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, diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a1a91cf9bd1..60d6a9e202c 100644 --- a/spec/models/user_spec.rb +++ b/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]) diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index f4f1b41a4e7..493a4cc599b 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/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 diff --git a/spec/script/import_scripts/vanilla_body_parser_spec.rb b/spec/script/import_scripts/vanilla_body_parser_spec.rb index 51e1106a9d8..31a2fdb3859 100644 --- a/spec/script/import_scripts/vanilla_body_parser_spec.rb +++ b/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
body
of the quoted discussion.
\n\n[/quote]\n\nWhen did this happen?" end diff --git a/spec/serializers/current_user_serializer_spec.rb b/spec/serializers/current_user_serializer_spec.rb index a8095e7557d..68479524d84 100644 --- a/spec/serializers/current_user_serializer_spec.rb +++ b/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 diff --git a/spec/serializers/user_card_serializer_spec.rb b/spec/serializers/user_card_serializer_spec.rb index 0e070cfcc57..94c6efe2330 100644 --- a/spec/serializers/user_card_serializer_spec.rb +++ b/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 diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb index b5abe360d24..05765453258 100644 --- a/spec/serializers/user_serializer_spec.rb +++ b/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