From a4d17414fddbfe75c1d876717dc41937d02bde28 Mon Sep 17 00:00:00 2001
From: Azul <azul@riseup.net>
Date: Thu, 17 Nov 2016 14:49:16 +0100
Subject: 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
---
app/models/user.rb | 2 +-
test/unit/account_test.rb | 48 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+), 1 deletion(-)
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
--
cgit v1.2.3