github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-22 10:57:04 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

WIP: more fixing

Browse Source
This commit is contained in:
Loïc Guitaut 2024-11-07 18:03:12 +01:00
parent 9cc7f7e1b8
commit e42a999799
No known key found for this signature in database
7 changed files with 104 additions and 128 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

82
spec/requests/users_controller_spec.rb
View File

@ -699,15 +699,16 @@ RSpec.describe UsersController do
params
end
let(:user) { Fabricate.build(:user, email: "foobar@example.com", password: "strongpassword") }
before do
UsersController.any_instance.stubs(:honeypot_value).returns(nil)
UsersController.any_instance.stubs(:challenge_value).returns(nil)
SiteSetting.allow_new_registrations = true
@user = Fabricate.build(:user, email: "foobar@example.com", password: "strongpassword")
end
let(:post_user_params) do
{ name: @user.name, username: @user.username, password: "strongpassword", email: @user.email }
{ name: user.name, username: user.username, password: "strongpassword", email: user.email }
end
def post_user(extra_params = {})
@ -718,8 +719,8 @@ RSpec.describe UsersController do
it "should raise the right error" do
post "/u.json",
params: {
name: @user.name,
username: @user.username,
name: user.name,
username: user.username,
password: "testing12352343",
}
expect(response.status).to eq(400)
@ -753,7 +754,7 @@ RSpec.describe UsersController do
SiteSetting.default_locale = "en"
I18n.stubs(:locale).returns(:fr)
post_user
expect(User.find_by(username: @user.username).locale).to eq("fr")
expect(User.find_by(username: user.username).locale).to eq("fr")
end
it "requires invite code when specified" do
@ -777,7 +778,7 @@ RSpec.describe UsersController do
it "sets the timezone" do
post_user(timezone: "Australia/Brisbane")
expect(response.status).to eq(200)
expect(User.find_by(username: @user.username).user_option.timezone).to eq(
expect(User.find_by(username: user.username).user_option.timezone).to eq(
"Australia/Brisbane",
)
end
@ -1376,7 +1377,7 @@ RSpec.describe UsersController do
expect(response.status).to eq(200)
json = response.parsed_body
expect(json["success"]).to eq(true)
expect(User.find_by(username: @user.username).active).to eq(false)
expect(User.find_by(username: user.username).active).to eq(false)
end
shared_examples "honeypot fails" do
@ -1400,10 +1401,10 @@ RSpec.describe UsersController do
before { UsersController.any_instance.stubs(:honeypot_value).returns("abc") }
let(:create_params) do
{
name: @user.name,
username: @user.username,
name: user.name,
username: user.username,
password: "strongpassword",
email: @user.email,
email: user.email,
password_confirmation: "wrong",
}
end
@ -1414,10 +1415,10 @@ RSpec.describe UsersController do
before { UsersController.any_instance.stubs(:challenge_value).returns("abc") }
let(:create_params) do
{
name: @user.name,
username: @user.username,
name: user.name,
username: user.username,
password: "strongpassword",
email: @user.email,
email: user.email,
challenge: "abc",
}
end
@ -1428,12 +1429,7 @@ RSpec.describe UsersController do
before { SiteSetting.invite_only = true }
let(:create_params) do
{
name: @user.name,
username: @user.username,
password: "strongpassword",
email: @user.email,
}
{ name: user.name, username: user.username, password: "strongpassword", email: user.email }
end
include_examples "honeypot fails"
@ -1458,7 +1454,7 @@ RSpec.describe UsersController do
context "when password is blank" do
let(:create_params) do
{ name: @user.name, username: @user.username, password: "", email: @user.email }
{ name: user.name, username: user.username, password: "", email: user.email }
end
include_examples "failed signup"
end
@ -1466,23 +1462,23 @@ RSpec.describe UsersController do
context "when password is too long" do
let(:create_params) do
{
name: @user.name,
username: @user.username,
name: user.name,
username: user.username,
password: "x" * (User.max_password_length + 1),
email: @user.email,
email: user.email,
}
end
include_examples "failed signup"
end
context "when password param is missing" do
let(:create_params) { { name: @user.name, username: @user.username, email: @user.email } }
let(:create_params) { { name: user.name, username: user.username, email: user.email } }
include_examples "failed signup"
end
context "with a reserved username" do
let(:create_params) do
{ name: @user.name, username: "Reserved", email: @user.email, password: "strongpassword" }
{ name: user.name, username: "Reserved", email: user.email, password: "strongpassword" }
end
before { SiteSetting.reserved_usernames = "a|reserved|b" }
include_examples "failed signup"
@ -1491,9 +1487,9 @@ RSpec.describe UsersController do
context "with a username that matches a user route" do
let(:create_params) do
{
name: @user.name,
name: user.name,
username: "account-created",
email: @user.email,
email: user.email,
password: "strongpassword",
}
end
@ -1501,7 +1497,7 @@ RSpec.describe UsersController do
end
context "with a missing username" do
let(:create_params) { { name: @user.name, email: @user.email, password: "x" * 20 } }
let(:create_params) { { name: user.name, email: user.email, password: "x" * 20 } }
it "should not create a new User" do
expect { post "/u.json", params: create_params }.to_not change { User.count }
@ -1513,12 +1509,7 @@ RSpec.describe UsersController do
before { User.any_instance.stubs(:save).raises(ActiveRecord::StatementInvalid.new("Oh no")) }
let(:create_params) do
{
name: @user.name,
username: @user.username,
password: "strongpassword",
email: @user.email,
}
{ name: user.name, username: user.username, password: "strongpassword", email: user.email }
end
include_examples "failed signup"
@ -1531,7 +1522,7 @@ RSpec.describe UsersController do
context "without a value for the fields" do
let(:create_params) do
{ name: @user.name, password: "watwatwat", username: @user.username, email: @user.email }
{ name: user.name, password: "watwatwat", username: user.username, email: user.email }
end
include_examples "failed signup"
end
@ -1676,10 +1667,10 @@ RSpec.describe UsersController do
let(:create_params) do
{
name: @user.name,
name: user.name,
password: "suChS3cuRi7y",
username: @user.username,
email: @user.email,
username: user.username,
email: user.email,
user_fields: {
user_field.id.to_s => "value1",
another_field.id.to_s => "value2",
@ -1690,7 +1681,7 @@ RSpec.describe UsersController do
it "should succeed without the optional field" do
post "/u.json", params: create_params
expect(response.status).to eq(200)
inserted = User.find_by_email(@user.email)
inserted = User.find_by_email(user.email)
expect(inserted).to be_present
expect(inserted.custom_fields).to be_present
expect(inserted.custom_fields["user_field_#{user_field.id}"]).to eq("value1")
@ -1702,7 +1693,7 @@ RSpec.describe UsersController do
create_params[:user_fields][optional_field.id.to_s] = "value3"
post "/u.json", params: create_params.merge(create_params)
expect(response.status).to eq(200)
inserted = User.find_by_email(@user.email)
inserted = User.find_by_email(user.email)
expect(inserted).to be_present
expect(inserted.custom_fields).to be_present
expect(inserted.custom_fields["user_field_#{user_field.id}"]).to eq("value1")
@ -1714,7 +1705,7 @@ RSpec.describe UsersController do
create_params[:user_fields][optional_field.id.to_s] = ("x" * 3000)
post "/u.json", params: create_params.merge(create_params)
expect(response.status).to eq(200)
inserted = User.find_by_email(@user.email)
inserted = User.find_by_email(user.email)
val = inserted.custom_fields["user_field_#{optional_field.id}"]
expect(val.length).to eq(UserField.max_length)
@ -1727,18 +1718,13 @@ RSpec.describe UsersController do
context "without values for the fields" do
let(:create_params) do
{
name: @user.name,
password: "suChS3cuRi7y",
username: @user.username,
email: @user.email,
}
{ name: user.name, password: "suChS3cuRi7y", username: user.username, email: user.email }
end
it "should succeed" do
post "/u.json", params: create_params
expect(response.status).to eq(200)
inserted = User.find_by_email(@user.email)
inserted = User.find_by_email(user.email)
expect(inserted).to be_present
expect(inserted.custom_fields).not_to be_present
expect(inserted.custom_fields["user_field_#{user_field.id}"]).to be_blank

39
spec/services/external_upload_manager_spec.rb
View File

@ -15,6 +15,7 @@ RSpec.describe ExternalUploadManager do
let(:external_upload_stub_metadata) { {} }
let!(:external_upload_stub) { Fabricate(:image_external_upload_stub, created_by: user) }
let(:s3_bucket_name) { SiteSetting.s3_upload_bucket }
let(:fake_s3) { FakeS3.create }
before do
SiteSetting.authorized_extensions += "|pdf"
@ -25,7 +26,12 @@ RSpec.describe ExternalUploadManager do
SiteSetting.s3_backup_bucket = "s3-backup-bucket"
SiteSetting.backup_location = BackupLocationSiteSetting::S3
prepare_fake_s3
fake_s3.bucket(s3_bucket_name).put_object(
key: external_upload_stub.key,
size: object_size,
last_modified: Time.zone.now,
metadata: external_upload_stub_metadata,
)
stub_download_object_filehelper
end
@ -73,8 +79,8 @@ RSpec.describe ExternalUploadManager do
it "copies the stubbed upload on S3 to its new destination and deletes it" do
upload = manager.transform!
bucket = @fake_s3.bucket(SiteSetting.s3_upload_bucket)
expect(@fake_s3.operation_called?(:copy_object)).to eq(true)
bucket = fake_s3.bucket(SiteSetting.s3_upload_bucket)
expect(fake_s3.operation_called?(:copy_object)).to eq(true)
expect(bucket.find_object(Discourse.store.get_path_for_upload(upload))).to be_present
expect(bucket.find_object(external_upload_stub.key)).to be_nil
end
@ -117,8 +123,8 @@ RSpec.describe ExternalUploadManager do
it "creates a new upload in s3 (not copy) and deletes the original stubbed upload" do
upload = manager.transform!
bucket = @fake_s3.bucket(SiteSetting.s3_upload_bucket)
expect(@fake_s3.operation_called?(:copy_object)).to eq(false)
bucket = fake_s3.bucket(SiteSetting.s3_upload_bucket)
expect(fake_s3.operation_called?(:copy_object)).to eq(false)
expect(bucket.find_object(Discourse.store.get_path_for_upload(upload))).to be_present
expect(bucket.find_object(external_upload_stub.key)).to be_nil
end
@ -136,7 +142,7 @@ RSpec.describe ExternalUploadManager do
)
expect(ExternalUploadStub.exists?(id: external_upload_stub.id)).to eq(false)
bucket = @fake_s3.bucket(SiteSetting.s3_upload_bucket)
bucket = fake_s3.bucket(SiteSetting.s3_upload_bucket)
expect(bucket.find_object(external_upload_stub.key)).to be_nil
end
@ -148,7 +154,7 @@ RSpec.describe ExternalUploadManager do
external_stub = ExternalUploadStub.find(external_upload_stub.id)
expect(external_stub.status).to eq(ExternalUploadStub.statuses[:failed])
bucket = @fake_s3.bucket(SiteSetting.s3_upload_bucket)
bucket = fake_s3.bucket(SiteSetting.s3_upload_bucket)
expect(bucket.find_object(external_upload_stub.key)).to be_present
end
end
@ -168,7 +174,7 @@ RSpec.describe ExternalUploadManager do
),
).to eq("1")
bucket = @fake_s3.bucket(SiteSetting.s3_upload_bucket)
bucket = fake_s3.bucket(SiteSetting.s3_upload_bucket)
expect(bucket.find_object(external_upload_stub.key)).to be_nil
end
@ -178,7 +184,7 @@ RSpec.describe ExternalUploadManager do
external_stub = ExternalUploadStub.find(external_upload_stub.id)
expect(external_stub.status).to eq(ExternalUploadStub.statuses[:failed])
bucket = @fake_s3.bucket(SiteSetting.s3_upload_bucket)
bucket = fake_s3.bucket(SiteSetting.s3_upload_bucket)
expect(bucket.find_object(external_upload_stub.key)).to be_present
end
end
@ -218,7 +224,7 @@ RSpec.describe ExternalUploadManager do
it "copies the stubbed upload on S3 to its new destination and deletes it" do
upload = manager.transform!
bucket = @fake_s3.bucket(SiteSetting.s3_upload_bucket)
bucket = fake_s3.bucket(SiteSetting.s3_upload_bucket)
expect(bucket.find_object(Discourse.store.get_path_for_upload(upload))).to be_present
expect(bucket.find_object(external_upload_stub.key)).to be_nil
end
@ -259,7 +265,7 @@ RSpec.describe ExternalUploadManager do
end
it "copies the stubbed upload on S3 to its new destination and deletes it" do
bucket = @fake_s3.bucket(SiteSetting.s3_backup_bucket)
bucket = fake_s3.bucket(SiteSetting.s3_backup_bucket)
expect(bucket.find_object(external_upload_stub.key)).to be_present
manager.transform!
@ -283,15 +289,4 @@ RSpec.describe ExternalUploadManager do
body: object_file.read,
)
end
def prepare_fake_s3
@fake_s3 = FakeS3.create
@fake_s3.bucket(s3_bucket_name).put_object(
key: external_upload_stub.key,
size: object_size,
last_modified: Time.zone.now,
metadata: external_upload_stub_metadata,
)
end
end

30
spec/services/user_destroyer_spec.rb
View File

@ -319,10 +319,9 @@ RSpec.describe UserDestroyer do
context "when user has posts with links" do
context "with external links" do
before do
@post = Fabricate(:post_with_external_links, user: user)
TopicLink.extract_from(@post)
end
let(:post) { Fabricate(:post_with_external_links, user: user) }
before { TopicLink.extract_from(post) }
it "doesn't add ScreenedUrl records by default" do
ScreenedUrl.expects(:watch).never
@ -336,9 +335,10 @@ RSpec.describe UserDestroyer do
end
context "with internal links" do
let(:post) { Fabricate(:post_with_external_links, user: user) }
before do
@post = Fabricate(:post_with_external_links, user: user)
TopicLink.extract_from(@post)
TopicLink.extract_from(post)
TopicLink.where(user: user).update_all(internal: true)
end
@ -349,10 +349,9 @@ RSpec.describe UserDestroyer do
end
context "with oneboxed links" do
before do
@post = Fabricate(:post_with_youtube, user: user)
TopicLink.extract_from(@post)
end
let(:post) { Fabricate(:post_with_youtube, user: user) }
before { TopicLink.extract_from(post) }
it "doesn't add ScreenedUrl records" do
ScreenedUrl.expects(:watch).never
@ -415,17 +414,16 @@ RSpec.describe UserDestroyer do
end
context "when user liked things" do
before do
@topic = Fabricate(:topic, user: Fabricate(:user))
@post = Fabricate(:post, user: @topic.user, topic: @topic)
PostActionCreator.like(user, @post)
end
let!(:topic) { Fabricate(:topic, user: Fabricate(:user)) }
let!(:post) { Fabricate(:post, user: topic.user, topic: topic) }
before { PostActionCreator.like(user, post) }
it "should destroy the like" do
expect { UserDestroyer.new(admin).destroy(user, delete_posts: true) }.to change {
PostAction.count
}.by(-1)
expect(@post.reload.like_count).to eq(0)
expect(post.reload.like_count).to eq(0)
end
end

10
spec/services/user_silencer_spec.rb
View File

@ -69,16 +69,16 @@ RSpec.describe UserSilencer do
end
context "with a plugin hook" do
before do
@override_silence_message = ->(opts) do
let(:override_silence_message) do
->(opts) do
opts[:silence_message_params][:message_title] = "override title"
opts[:silence_message_params][:message_raw] = "override raw"
end
DiscourseEvent.on(:user_silenced, &@override_silence_message)
end
after { DiscourseEvent.off(:user_silenced, &@override_silence_message) }
before { DiscourseEvent.on(:user_silenced, &override_silence_message) }
after { DiscourseEvent.off(:user_silenced, &override_silence_message) }
it "allows the message to be overridden" do
UserSilencer.silence(user, admin)

21
spec/services/user_stat_count_updater_spec.rb
View File

@ -6,32 +6,33 @@ RSpec.describe UserStatCountUpdater do
fab!(:post)
fab!(:post_2) { Fabricate(:post, topic: post.topic) }
let(:fake_logger) { FakeLogger.new }
before do
@orig_logger = Rails.logger
Rails.logger = @fake_logger = FakeLogger.new
Rails.logger.broadcast_to(fake_logger)
SiteSetting.verbose_user_stat_count_logging = true
end
after { Rails.logger = @orig_logger }
after { Rails.logger.stop_broadcasting_to(fake_logger) }
it "should log the exception when a negative count is inserted" do
UserStatCountUpdater.decrement!(post, user_stat: user_stat)
expect(@fake_logger.warnings.last).to match("topic_count")
expect(@fake_logger.warnings.last).to match(post.id.to_s)
expect(fake_logger.warnings.last).to match("topic_count")
expect(fake_logger.warnings.last).to match(post.id.to_s)
UserStatCountUpdater.decrement!(post_2, user_stat: user_stat)
expect(@fake_logger.warnings.last).to match("post_count")
expect(@fake_logger.warnings.last).to match(post_2.id.to_s)
expect(fake_logger.warnings.last).to match("post_count")
expect(fake_logger.warnings.last).to match(post_2.id.to_s)
end
it "should log the exception when a negative count will be inserted but 0 is used instead" do
UserStatCountUpdater.set!(user_stat: user_stat, count: -10, count_column: :post_count)
expect(@fake_logger.warnings.last).to match("post_count")
expect(@fake_logger.warnings.last).to match("using 0")
expect(@fake_logger.warnings.last).to match("user #{user_stat.user_id}")
expect(fake_logger.warnings.last).to match("post_count")
expect(fake_logger.warnings.last).to match("using 0")
expect(fake_logger.warnings.last).to match("user #{user_stat.user_id}")
expect(user_stat.reload.post_count).to eq(0)
end
end

25
spec/services/username_changer_spec.rb
View File

@ -12,12 +12,13 @@ RSpec.describe UsernameChanger do
it "should change the username" do
new_username = "#{user.username}1234"
result = nil
events =
DiscourseEvent
.track_events { @result = UsernameChanger.change(user, new_username) }
.track_events { result = UsernameChanger.change(user, new_username) }
.last(2)
expect(@result).to eq(true)
expect(result).to eq(true)
event = events.first
expect(event[:event_name]).to eq(:username_changed)
@ -36,10 +37,10 @@ RSpec.describe UsernameChanger do
it "do nothing if the new username is the same" do
new_username = user.username
events =
DiscourseEvent.track_events { @result = UsernameChanger.change(user, new_username) }
result = nil
events = DiscourseEvent.track_events { result = UsernameChanger.change(user, new_username) }
expect(@result).to eq(false)
expect(result).to eq(false)
expect(events.count).to be_zero
end
end
@ -50,8 +51,7 @@ RSpec.describe UsernameChanger do
let(:username_lower_before_change) { user.username_lower }
it "should not change the username" do
@result = UsernameChanger.change(user, wrong_username)
expect(@result).to eq(false)
expect(UsernameChanger.change(user, wrong_username)).to eq(false)
user.reload
expect(user.username).to eq(username_before_change)
@ -82,18 +82,17 @@ RSpec.describe UsernameChanger do
end
describe "allow custom minimum username length from site settings" do
before do
@custom_min = 2
SiteSetting.min_username_length = @custom_min
end
let(:custom_min) { 2 }
before { SiteSetting.min_username_length = custom_min }
it "should allow a shorter username than default" do
result = UsernameChanger.change(user, "a" * @custom_min)
result = UsernameChanger.change(user, "a" * custom_min)
expect(result).not_to eq(false)
end
it "should not allow a shorter username than limit" do
result = UsernameChanger.change(user, "a" * (@custom_min - 1))
result = UsernameChanger.change(user, "a" * (custom_min - 1))
expect(result).to eq(false)
end

25
spec/services/username_checker_service_spec.rb
View File

@ -2,30 +2,27 @@
RSpec.describe UsernameCheckerService do
describe "#check_username" do
before do
@service = UsernameCheckerService.new
@nil_email = nil
@email = "vincentvega@example.com"
end
let(:service) { UsernameCheckerService.new }
let(:email) { nil }
context "when username is invalid" do
it "rejects too short usernames" do
result = @service.check_username("a", @nil_email)
result = service.check_username("a", email)
expect(result).to have_key(:errors)
end
it "rejects too long usernames" do
result = @service.check_username("a123456789b123456789c123456789", @nil_email)
result = service.check_username("a123456789b123456789c123456789", email)
expect(result).to have_key(:errors)
end
it "rejects usernames with invalid characters" do
result = @service.check_username("vincent-", @nil_email)
result = service.check_username("vincent-", email)
expect(result).to have_key(:errors)
end
it "rejects usernames that do not start with an alphanumeric character" do
result = @service.check_username(".vincent", @nil_email)
result = service.check_username(".vincent", email)
expect(result).to have_key(:errors)
end
@ -33,13 +30,13 @@ RSpec.describe UsernameCheckerService do
before { SiteSetting.reserved_usernames = "test|donkey" }
it "rejects usernames that are reserved" do
result = @service.check_username("test", @nil_email)
result = service.check_username("test", email)
expect(result[:available]).to eq(false)
end
it "allows reserved username checker to be skipped" do
@service = UsernameCheckerService.new(allow_reserved_username: true)
result = @service.check_username("test", @nil_email)
service = UsernameCheckerService.new(allow_reserved_username: true)
result = service.check_username("test", email)
expect(result[:available]).to eq(true)
end
end
@ -48,14 +45,14 @@ RSpec.describe UsernameCheckerService do
it "username not available locally" do
User.stubs(:username_available?).returns(false)
UserNameSuggester.stubs(:suggest).returns("einar-j")
result = @service.check_username("vincent", @nil_email)
result = service.check_username("vincent", email)
expect(result[:available]).to eq(false)
expect(result[:suggestion]).to eq("einar-j")
end
it "username available locally" do
User.stubs(:username_available?).returns(true)
result = @service.check_username("vincent", @nil_email)
result = service.check_username("vincent", email)
expect(result[:available]).to eq(true)
end
end