Skip to content

Commit

Permalink
Found a simpler/cleaner way to do this
Browse files Browse the repository at this point in the history
Thankfully, no longer requires disgusting hacking into
devise. Instead just a tri-state boolean, that can never be false.*

* Simples.
  • Loading branch information
pkqk committed Jan 20, 2015
1 parent 3daab10 commit 576915b
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 30 deletions.
30 changes: 2 additions & 28 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ class User < ActiveRecord::Base

before_save :ensure_authentication_token

validates :agreed_to_terms, acceptance: {accept: true}, allow_nil: true

def ensure_authentication_token
if authentication_token.blank?
self.authentication_token = generate_authentication_token
Expand All @@ -29,34 +31,6 @@ def after_password_reset
confirm!
end

# Coped from https://github.com/plataformatec/devise/blob/v3.0.3/lib/devise/models/database_authenticatable.rb
# Sorry but I couldn't add a check around this method as .valid? wipes
# other errors from the hash
def update_with_password(params, *options)
current_password = params.delete(:current_password)

if params[:password].blank?
params.delete(:password)
params.delete(:password_confirmation) if params[:password_confirmation].blank?
end

# special case for rails handling of boolean check_box
agreed_to_terms = params[:agreed_to_terms] == "1"

result = if valid_password?(current_password) && agreed_to_terms
update_attributes(params, *options)
else
self.assign_attributes(params, *options)
self.valid?
self.errors.add(:current_password, current_password.blank? ? :blank : :invalid) unless valid_password?(current_password)
self.errors.add(:agreed_to_terms, :blank) unless agreed_to_terms?
false
end

clean_up_passwords
result
end

def to_s
if name.present?
"#{name} <#{email}>"
Expand Down
2 changes: 1 addition & 1 deletion db/migrate/20150119114745_add_extra_user_details.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class AddExtraUserDetails < ActiveRecord::Migration
def up
add_column :users, :organization, :string
add_column :users, :agreed_to_terms, :boolean, null: false, default: false
add_column :users, :agreed_to_terms, :boolean, null: true, default: nil
end

def down
Expand Down
2 changes: 1 addition & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@
t.datetime "confirmation_sent_at"
t.string "unconfirmed_email"
t.string "organization"
t.boolean "agreed_to_terms", :default => false, :null => false
t.boolean "agreed_to_terms"
end

add_index "users", ["authentication_token"], :name => "index_users_on_authentication_token", :unique => true
Expand Down
12 changes: 12 additions & 0 deletions test/unit/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,16 @@ class UserTest < ActiveSupport::TestCase
assert_equal "Joan Jett <joan@example.com>", user.to_s
end

test "can create a user without specifying agreement to terms" do
assert User.new(email: 'robyn@example.com', password: 'password').valid?
end

test "can create a user that agreeds to terms" do
assert User.new(email: 'robyn@example.com', password: 'password', agreed_to_terms: true).valid?
end

test "not agreeing to terms prevents saving" do
refute User.new(email: 'robyn@example.com', password: 'password', agreed_to_terms: false).valid?
end

end

0 comments on commit 576915b

Please sign in to comment.