diff options
author | Azul <azul@riseup.net> | 2016-11-17 14:49:16 +0100 |
---|---|---|
committer | Azul <azul@riseup.net> | 2016-11-17 14:49:16 +0100 |
commit | a4d17414fddbfe75c1d876717dc41937d02bde28 (patch) | |
tree | 862898f476dccd5495be996a47492ef439ba3cab | |
parent | 47c6bc5e62df9a7ad07cd975ea07113f6df4fedc (diff) |
bugfix: only send login errors once
If a login was invalid as a username but also for the identity we used to have duplicate error messages. Let's avoid that.
Also added a test to make sure invite_code errors are properly displayed no matter what other fields are set or missing. Pixelated will rely on this to test invite codes
-rw-r--r-- | app/models/user.rb | 2 | ||||
-rw-r--r-- | test/unit/account_test.rb | 48 |
2 files changed, 49 insertions, 1 deletions
diff --git a/app/models/user.rb b/app/models/user.rb index 9cebbca..259778b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -230,7 +230,7 @@ class User < CouchRest::Model::Base def identity_is_valid return if identity.valid? identity.errors.each do |attribute, error| - self.errors.add(:login, error) + errors.add(:login, error) unless errors[:login].include? error end end diff --git a/test/unit/account_test.rb b/test/unit/account_test.rb index d56541a..e00e589 100644 --- a/test/unit/account_test.rb +++ b/test/unit/account_test.rb @@ -26,6 +26,7 @@ class AccountTest < ActiveSupport::TestCase user = Account.create(FactoryGirl.attributes_for(:user)) 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" end end @@ -47,6 +48,25 @@ class AccountTest < ActiveSupport::TestCase end end + test "error on reused username" do + with_config invite_required: false do + attributes = FactoryGirl.attributes_for :user + user = Account.create attributes + dup = Account.create attributes + assert !dup.valid? + assert_has_errors dup, login: "has already been taken" + user.account.destroy + end + end + + test "error on invalid username" do + with_config invite_required: false do + attributes = FactoryGirl.attributes_for :user, login: "a" + user = Account.create attributes + assert !user.valid? + assert_has_errors user, login: "Must have at least two characters" + end + end test "create and remove a user account" do # We keep an identity that will block the handle from being reused. @@ -110,4 +130,32 @@ class AccountTest < ActiveSupport::TestCase user.account.enable assert_equal(cert.fingerprint, Identity.for(user).cert_fingerprints.keys.first) end + + # Pixelated relies on the ability to test invite codes without sending a + # username and password yet. + # So we better make sure we return the appropriate errors + test "errors trying to create account with invite only" do + with_config invite_required: true do + user = Account.create invite_code: @testcode.invite_code + assert user.errors[:invite_code].blank? + end + end + + test "errors trying to create account with invalid invite only" do + with_config invite_required: true do + user = Account.create invite_code: "wrong_invite_code" + assert_has_errors user, invite_code: "This is not a valid code" + end + end + + # 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) + errors.each do |field, field_errors| + Array(field_errors).each do |error| + assert_includes record.errors[field], error + end + end + end + end |