Merge pull request #2431 from TracksApp/user_email_address

User email address

app/assets/stylesheets/login.scss

@@ -11,6 +11,9 @@ body {
 
 .login-wrapper {
   @include make-row();
+  .footer {
+    z-index: 1;
+  }
 }
 
 .login-box {

app/controllers/preferences_controller.rb

@@ -44,7 +44,7 @@ private
   end
 
   def user_params
-    params.require(:user).permit(:login, :first_name, :last_name, :password_confirmation, :password, :auth_type, :open_id_url)
+    params.require(:user).permit(:login, :first_name, :last_name, :email, :password_confirmation, :password, :auth_type, :open_id_url)
   end
 
   # Display notification if preferences are successful updated

app/controllers/users_controller.rb

@@ -187,7 +187,7 @@ class UsersController < ApplicationController
   private
 
   def user_params
-    params.require(:user).permit(:login, :first_name, :last_name, :password_confirmation, :password, :auth_type, :open_id_url)
+    params.require(:user).permit(:login, :first_name, :last_name, :email, :password_confirmation, :password, :auth_type, :open_id_url)
   end
 
   def get_new_user

app/models/user.rb

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

app/views/preferences/_profile.html.erb

@@ -4,6 +4,9 @@
 <div class="form-group">
   <%= pref_with_text_field 'user', 'last_name' %>
 </div>
+<div class="form-group">
+  <%= pref_with_text_field 'user', 'email' %>
+</div>
 <div class="form-group">
   <%= pref_with_select_field('prefs', 'locale', I18n.available_locales.map {|l| l.to_s}) %>
 </div>

app/views/users/new.html.erb

@@ -22,6 +22,10 @@
             <%= label_tag "user_login", t('users.desired_login') %>
             <%= text_field "user", "login", class: "form-control" %>
           </div>
+          <div class="label-element-combo">
+            <%= label_tag "user_email", t('users.email_address') %>
+            <%= text_field "user", "email", class: "form-control" %>
+          </div>
           <div class="label-element-combo">
             <%= label_tag "user_password", t('users.choose_password') %>
             <%= password_field "user", "password", class: "form-control" %>

config/locales/en.yml

@@ -51,6 +51,7 @@ en:
       user:
         auth_type: Auth type
         display_name: Display name
+        email: Email address
         first_name: First name
         last_name: Last name
         login: Login
@@ -87,7 +88,7 @@ en:
         confirmation: doesn't match confirmation
         less_than_or_equal_to: must be less than or equal to %{count}
         blank: can't be blank
-        invalid: "cannot contain the comma (',') character"
+        invalid: "is not valid"
         exclusion: is reserved
         odd: must be odd
         even: must be even
@@ -961,6 +962,7 @@ en:
     change_password_prompt: Enter your new password in the fields below and click 'Change password' to replace your current password with your new one.
     password_confirmation_label: Confirm password
     destroy_error: There was an error deleting the user %{login}
+    email_address: Email address
     choose_password: Choose password
     register_with_cas: With your CAS username
     label_auth_type: Authentication type

db/migrate/20200720151220_add_email_to_user.rb

@@ -0,0 +1,5 @@
+class AddEmailToUser < ActiveRecord::Migration[5.2]
+  def change
+    add_column :users, :email, :string
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2020_01_09_231555) do
+ActiveRecord::Schema.define(version: 2020_07_20_151220) do
 
   create_table "attachments", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t|
     t.integer "todo_id"
@@ -220,6 +220,7 @@ ActiveRecord::Schema.define(version: 2020_01_09_231555) do
     t.string "open_id_url"
     t.string "remember_token"
     t.datetime "remember_token_expires_at"
+    t.string "email"
     t.index ["login"], name: "index_users_on_login"
   end
 

test/controllers/users_controller_test.rb

@@ -18,7 +18,7 @@ class UsersControllerTest < ActionController::TestCase
     get :index
     assert_response :success
     assert_equal "TRACKS::Manage Users", assigns['page_title']
-    assert_equal 4, assigns['total_users']
+    assert_equal 5, assigns['total_users']
     assert_equal users_url, session['return-to']
   end
 

test/fixtures/users.yml

@@ -48,3 +48,15 @@ ldap_user:
   first_name: International
   last_name: Harvester
   auth_type: CAS
+
+other_user_email:
+  id: 6
+  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
+

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,16 +63,27 @@ 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"
+    barracuda2 = 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
     assert_select 'users' do
-      assert_select 'user', count: 4
+      assert_select 'user', count: 5
     end
     assert_select 'password', false
   end
@@ -81,7 +94,15 @@ class UsersXmlApiTest < ActionDispatch::IntegrationTest
     assert_select 'user'
     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 +112,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"