From 9414e8ddf4ce7e951a6dc9130d99fa12c5696b94 Mon Sep 17 00:00:00 2001 From: Azul Date: Tue, 17 Oct 2017 11:05:53 +0200 Subject: fix: destroy invites used to create test accounts Production instances are getting cluttered with invites from test accounts. Instead of marking them as used we will now completely remove them. refers to #8804 refers to #8807\ --- app/models/account.rb | 8 ++++++-- test/unit/account_test.rb | 45 ++++++++++++++++++++++++++++++--------------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index 3283bcc..4442a68 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -43,8 +43,12 @@ class Account end if user.invite_required? user_invite_code = InviteCode.find_by_invite_code user.invite_code - user_invite_code.invite_count += 1 - user_invite_code.save + if user.is_test? && user_invite_code.max_uses == 1 + user_invite_code.destroy + else + user_invite_code.invite_count += 1 + user_invite_code.save + end end end rescue VALIDATION_FAILED => ex diff --git a/test/unit/account_test.rb b/test/unit/account_test.rb index 058e196..f81764d 100644 --- a/test/unit/account_test.rb +++ b/test/unit/account_test.rb @@ -12,7 +12,7 @@ class AccountTest < ActiveSupport::TestCase end test "create a new account when invited" do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) assert user.valid?, "unexpected errors: #{user.errors.inspect}" assert user.persisted? assert id = user.identity @@ -23,7 +23,7 @@ class AccountTest < ActiveSupport::TestCase test "fail to create account without invite" do with_config invite_required: true do - user = Account.create(FactoryGirl.attributes_for(:user)) + user = Account.create(user_attributes) assert !user.valid?, "user should not be valid" assert !user.persisted?, "user should not have been saved" assert_has_errors user, invite_code: "This is not a valid code" @@ -32,7 +32,7 @@ class AccountTest < ActiveSupport::TestCase test "allow invite_required override" do with_config invite_required: true do - user = Account.create(FactoryGirl.attributes_for(:user), :invite_required => false) + user = Account.create(user_attributes, :invite_required => false) assert user.valid?, "unexpected errors: #{user.errors.inspect}" assert user.persisted?, "user should have been saved" user.account.destroy @@ -41,7 +41,7 @@ class AccountTest < ActiveSupport::TestCase test "create a new account" do with_config invite_required: false do - user = Account.create(FactoryGirl.attributes_for(:user)) + user = Account.create(user_attributes) assert user.valid?, "unexpected errors: #{user.errors.inspect}" assert user.persisted? user.account.destroy @@ -50,7 +50,7 @@ class AccountTest < ActiveSupport::TestCase test "error on reused username" do with_config invite_required: false do - attributes = FactoryGirl.attributes_for :user + attributes = user_attributes user = Account.create attributes dup = Account.create attributes assert !dup.valid? @@ -72,17 +72,17 @@ class AccountTest < ActiveSupport::TestCase # We keep an identity that will block the handle from being reused. assert_difference "Identity.count" do assert_no_difference "User.count" do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) user.account.destroy end end end test "change username and create alias" do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) old_id = user.identity old_email = user.email_address - user.account.update(FactoryGirl.attributes_for(:user)) + user.account.update(user_attributes) user.reload old_id.reload assert user.valid? @@ -97,7 +97,7 @@ class AccountTest < ActiveSupport::TestCase end test "create recovery code if it does not exist" do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) user.account.update(:recovery_code_verifier => "abc", :recovery_code_salt => "123") user.reload @@ -108,7 +108,7 @@ class AccountTest < ActiveSupport::TestCase end test "update recovery code that already exists" do - user = Account.create(FactoryGirl.attributes_for(:user, + user = Account.create(user_attributes( :invite_code => @testcode.invite_code, :recovery_code_verifier => "000", :recovery_code_salt => "111")) @@ -123,7 +123,7 @@ class AccountTest < ActiveSupport::TestCase end test "update password" do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) user.account.update(:password_verifier => "551A8B", :password_salt => "551A8B") assert_equal "551A8B", user.password_verifier @@ -134,18 +134,27 @@ class AccountTest < ActiveSupport::TestCase test "Invite code count goes up by 1 when the invite code is entered" do with_config invite_required: true do - user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) user_code = InviteCode.find_by_invite_code user.invite_code - user_code.save user.save assert user.persisted? assert_equal 1, user_code.invite_count end + end + test "Single use invite code is destroyed when used by test user" do + with_config invite_required: true do + attrs = user_attributes invite_code: @testcode.invite_code + attrs[:login] = 'test_user_' + attrs[:login] + user = Account.create(attrs) + user.save + assert user.persisted?, user.errors.inspect + assert_nil InviteCode.find_by_invite_code user.invite_code + end end test "Invite code stays zero when invite code is not used" do - #user = Account.create(FactoryGirl.attributes_for(:user, :invite_code => @testcode.invite_code)) + #user = Account.create(user_attributes( :invite_code => @testcode.invite_code)) invalid_user = FactoryGirl.build(:user, :invite_code => @testcode.invite_code) invalid_user.save user_code = InviteCode.find_by_invite_code invalid_user.invite_code @@ -155,7 +164,7 @@ class AccountTest < ActiveSupport::TestCase end test "disabled accounts have no cert fingerprints" do - user = Account.create(FactoryGirl.attributes_for(:user)) + user = Account.create(user_attributes) cert = stub(expiry: 1.month.from_now, fingerprint: SecureRandom.hex) user.identity.register_cert cert user.identity.save @@ -184,6 +193,8 @@ class AccountTest < ActiveSupport::TestCase end end + protected + # Tests for the presence of the errors given. # Does not test for the absence of other errors - so there may be more. def assert_has_errors(record, errors) @@ -194,4 +205,8 @@ class AccountTest < ActiveSupport::TestCase end end + def user_attributes(attrs = {}) + FactoryGirl.attributes_for :user, attrs + end + end -- cgit v1.2.3