Make email address voluntary in preferences; update tests to handle the added email field better

app/models/user.rb

@@ -107,7 +107,7 @@ class User < ApplicationRecord
   validates_length_of :login, within: 3..80
   validates_uniqueness_of :login, on: :create
   validate :validate_auth_type
-  validates :email, format: { with: URI::MailTo::EMAIL_REGEXP }
+  validates :email, :allow_blank => true, format: { with: URI::MailTo::EMAIL_REGEXP }
 
   before_create :crypt_password, :generate_token
   before_update :crypt_password

test/fixtures/users.yml

@@ -19,6 +19,17 @@ other_user:
   last_name: Doe
   auth_type: database
 
+other_user_email:
+  id: 3
+  login: joe
+  crypted_password: <%= BCrypt::Password.create("open") %>
+  token: <%= Digest::SHA1.hexdigest("joeSun Feb 19 14:42:45 GMT 20060.408173979260027") %>
+  is_admin: false
+  first_name: Jane
+  last_name: Doe
+  email: joe@example.org
+  auth_type: database
+
 ldap_user:
   id: 3
   login: john

test/integration/stories_test.rb

@@ -3,7 +3,7 @@ require 'support/stub_site_config_helper'
 
 class StoriesTest < ActionDispatch::IntegrationTest
   include StubSiteConfigHelper
-  
+
   # ####################################################
   # Testing login and signup by different kinds of users
   # ####################################################
@@ -11,10 +11,11 @@ class StoriesTest < ActionDispatch::IntegrationTest
     admin = new_session_as(:admin_user,"abracadabra")
     admin.goes_to_signup
     admin.signs_up_with(:user => {:login => "newbie",
+                                  :email => "test.person@example.org",
                                   :password => "newbiepass",
                                   :password_confirmation => "newbiepass"})
   end
-  
+
   def test_signup_new_user_by_nonadmin
     stub_site_config do
       SITE_CONFIG['open_signups'] = false
@@ -22,7 +23,7 @@ class StoriesTest < ActionDispatch::IntegrationTest
       other_user.goes_to_signup_as_nonadmin
     end
   end
-  
+
   def test_open_signup_new_user
     stub_site_config do
       SITE_CONFIG['open_signups'] = true
@@ -30,6 +31,7 @@ class StoriesTest < ActionDispatch::IntegrationTest
       assert_response :success
       assert_template "users/new"
       post "/users", params: { :user => {:login => "newbie",
+                                         :email => "test.person@example.org",
                                          :password => "newbiepass",
                                          :password_confirmation => "newbiepass"} }
       assert_response :redirect
@@ -37,8 +39,8 @@ class StoriesTest < ActionDispatch::IntegrationTest
       assert_response :success
       assert_template "todos/index"
     end
-  end    
+  end
-  
+
   private
 
     module CustomAssertions
@@ -67,7 +69,7 @@ class StoriesTest < ActionDispatch::IntegrationTest
         assert_response :success
         assert_template "users/new"
       end
-      
+
       def goes_to_signup_as_nonadmin
         get "/signup"
         assert_response :success
@@ -81,7 +83,6 @@ class StoriesTest < ActionDispatch::IntegrationTest
         assert_response :success
         assert_template "todos/index"
       end
-      
     end
 
     def new_session_as(user,plainpass)
@@ -92,5 +93,4 @@ class StoriesTest < ActionDispatch::IntegrationTest
         yield sess if block_given?
       end
     end
-    
 end

test/integration/users_xml_api_test.rb

@@ -1,20 +1,22 @@
 require 'test_helper'
 
 class UsersXmlApiTest < ActionDispatch::IntegrationTest
-  
+
   @@foobar_postdata = "<user><login>foo</login><password>bar</password></user>"
+  @@barfoo_postdata = "<user><login>bar</login><email>barfoo@example.org</email><password>foo</password></user>"
   @@johnny_postdata = "<user><login>johnny</login><password>barracuda</password></user>"
-  
+  @@barracuda_postdata = "<user><login>barracuda</login><email>barracuda@example.org</email><password>johnny</password></user>"
+
   def test_fails_with_401_if_not_authorized_user
     authenticated_post_xml_to_user_create @@foobar_postdata, 'nobody', 'nohow'
     assert_401_unauthorized_admin
   end
-  
+
  def test_fails_with_401_if_not_admin_user
    authenticated_post_xml_to_user_create @@foobar_postdata, users(:other_user).login, 'sesame'
    assert_401_unauthorized_admin
  end
- 
+
  def test_content_type_must_be_xml
    authenticated_post_xml_to_user_create @@foobar_postdata, users(:admin_user).login, 'abracadabra', {'CONTENT_TYPE' => "application/x-www-form-urlencoded"}
    assert_response 400, "Expected response 400"
@@ -25,12 +27,12 @@ class UsersXmlApiTest < ActionDispatch::IntegrationTest
  #   authenticated_post_xml_to_user_create "<foo></bar>"
  #   assert_equal 500, @integration_session.status
  # end
-    
+
   def test_fails_with_invalid_xml_format2
     authenticated_post_xml_to_user_create "<username>foo</username>"
     assert_response_and_body 400, "Expected post format is valid xml like so: <user><login>username</login><password>abc123</password></user>."
   end
-  
+
   def test_xml_simple_param_parsing
     authenticated_post_xml_to_user_create
     assert @controller.params.has_key?(:user)
@@ -39,18 +41,18 @@ class UsersXmlApiTest < ActionDispatch::IntegrationTest
     assert_equal 'foo', @controller.params['user'][:login]
     assert_equal 'bar', @controller.params['user'][:password]
   end
-  
+
   def test_fails_with_too_short_password
     authenticated_post_xml_to_user_create
     assert_responses_with_error "Password is too short (minimum is 5 characters"
   end
-  
+
   def test_fails_with_nonunique_login
     existing_login = users(:other_user).login
     authenticated_post_xml_to_user_create "<user><login>#{existing_login}</login><password>barracuda</password></user>"
     assert_responses_with_error "Login has already been taken"
   end
-  
+
   def test_creates_new_user
     assert_difference 'User.count' do
       authenticated_post_xml_to_user_create @@johnny_postdata
@@ -61,11 +63,22 @@ class UsersXmlApiTest < ActionDispatch::IntegrationTest
     johnny2 = User.authenticate('johnny','barracuda')
     assert_not_nil johnny2, "expected user johnny to be authenticated"
   end
-  
+
+  def test_creates_new_user
+    assert_difference 'User.count' do
+      authenticated_post_xml_to_user_create @@barracuda_postdata
+      assert_response_and_body 200, "User created."
+    end
+    barracuda1 = User.where(:login => 'barracuda').first
+    assert_not_nil barracuda1, "expected user barracuda to be created"
+    johnny2 = User.authenticate('barracuda','johnny')
+    assert_not_nil barracuda2, "expected user barracuda to be authenticated"
+  end
+
   def test_fails_with_get_verb
     authenticated_get_xml "/users.xml", users(:admin_user).login, 'abracadabra', {}
   end
-  
+
   def test_get_users_as_xml
     get '/users.xml', params: {}, headers: basic_auth_headers()
     assert_response :success
@@ -79,9 +92,18 @@ class UsersXmlApiTest < ActionDispatch::IntegrationTest
     get "/users/#{users(:other_user).id}.xml", params: {}, headers: basic_auth_headers()
     assert_response :success
     assert_select 'user'
+    assert_select 'email', false
     assert_select 'password', false
   end
-    
+
+  def test_get_email_user_as_xml
+    get "/users/#{users(:other_user_email).id}.xml", params: {}, headers: basic_auth_headers()
+    assert_response :success
+    assert_select 'user'
+    assert_select 'email'
+    assert_select 'password', false
+  end
+
   private
 
   def basic_auth_headers(username = users(:admin_user).login, password = 'abracadabra')
@@ -91,4 +113,8 @@ class UsersXmlApiTest < ActionDispatch::IntegrationTest
   def authenticated_post_xml_to_user_create(postdata = @@foobar_postdata, user = users(:admin_user).login, password = 'abracadabra', headers = {})
     authenticated_post_xml "/users.xml", user, password, postdata, headers
   end
+
+  def authenticated_post_xml_to_user_create_with_email(postdata = @@barfoo_postdata, user = users(:admin_user).login, password = 'abracadabra', headers = {})
+    authenticated_post_xml "/users.xml", user, password, postdata, headers
+  end
 end

test/models/user_test.rb

@@ -89,6 +89,19 @@ class UserTest < ActiveSupport::TestCase
     end
   end
 
+  def test_validate_correct_email
+    assert_difference 'User.count' do
+      create_user :email=> 'testi@example.org'
+    end
+  end
+
+  def test_validate_email_format
+    assert_no_difference 'User.count' do
+      u = create_user :email=> 'test'
+      assert_equal "is not valid", u.errors[:email]
+    end
+  end
+
   def test_display_name_with_first_and_last_name_set
     @other_user.first_name = "Jane"
     @other_user.last_name = "Doe"