summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAzul <azul@leap.se>2012-12-18 13:41:41 +0100
committerAzul <azul@leap.se>2012-12-18 13:41:41 +0100
commit285b23f39765d8346a658a81eca8b70d70b3e9bf (patch)
treeddb8f4d433c6f0461f7832ad08475b9a5bae99b2
parent6e8a45145722c12dee4d15b33cc28d2b09881e1a (diff)
refactored email_alias creation and validation
using CouchRests user.email_aliases.build so the casted_by method is set in the alias Used this to move the validations into the alias itself. This is where they belong and allows us to render the errors inline along the email field they belong to.
-rw-r--r--users/app/controllers/users_controller.rb6
-rw-r--r--users/app/models/local_email.rb21
-rw-r--r--users/app/models/user.rb26
-rw-r--r--users/app/views/emails/_email.html.haml11
-rw-r--r--users/app/views/users/_email_aliases.html.haml2
-rw-r--r--users/test/unit/email_aliases_test.rb11
-rw-r--r--users/test/unit/email_test.rb6
7 files changed, 44 insertions, 39 deletions
diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb
index 8ba6a7b..5c6767c 100644
--- a/users/app/controllers/users_controller.rb
+++ b/users/app/controllers/users_controller.rb
@@ -27,16 +27,16 @@ class UsersController < ApplicationController
end
def edit
+ @email_alias = LocalEmail.new
end
def update
@user.attributes = params[:user]
+ @email_alias = @user.email_aliases.last
if @user.changed? and @user.save
flash[:notice] = t(:user_updated_successfully)
- else
- flash.now[:error] = @user.errors.full_messages.to_sentence
end
- respond_with @user.reload, :location => edit_user_path(@user, :anchor => :email)
+ respond_with @user, :location => edit_user_path(@user, :anchor => :email)
end
def destroy
diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb
index 7cca4f4..c654fcb 100644
--- a/users/app/models/local_email.rb
+++ b/users/app/models/local_email.rb
@@ -1,6 +1,9 @@
class LocalEmail < Email
validate :unique_on_server
+ validate :unique_alias_for_user
+ validate :differs_from_main_email
+ validates :casted_by, :presence => true
def to_partial_path
"emails/email"
@@ -9,7 +12,23 @@ class LocalEmail < Email
def unique_on_server
has_email = User.find_by_email_or_alias(email)
if has_email && has_email != self.base_doc
- errors.add(:email, "has already been taken")
+ errors.add :email, "has already been taken"
end
end
+
+ def unique_alias_for_user
+ aliases = self.casted_by.email_aliases
+ if aliases.select{|a|a.email == self.email}.count > 1
+ errors.add :email, "is already your alias"
+ end
+ end
+
+ def differs_from_main_email
+ user = self.casted_by
+ if user.email == self.email
+ errors.add :email, "may not be the same as your email address"
+ end
+ end
+
+
end
diff --git a/users/app/models/user.rb b/users/app/models/user.rb
index d66b0e9..0773f28 100644
--- a/users/app/models/user.rb
+++ b/users/app/models/user.rb
@@ -35,9 +35,7 @@ class User < CouchRest::Model::Base
validates :email_forward,
:format => { :with => /\A(([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,}))?\Z/, :message => "needs to be a valid email address"}
- validate :no_duplicate_email_aliases
-
- validate :email_aliases_differ_from_email
+ validate :email_differs_from_email_aliases
timestamps!
@@ -117,34 +115,20 @@ class User < CouchRest::Model::Base
APP_CONFIG['admins'].include? self.login
end
- def add_email_alias(email)
- email = LocalEmail.new(email) unless email.is_a? Email
- email_aliases << email
- end
-
# this currently only adds the first email address submitted.
# All the ui needs for now.
def email_aliases_attributes=(attrs)
- if attrs && attrs.values.first
- add_email_alias attrs.values.first
- end
+ email_aliases.build(attrs.values.first) if attrs
end
-
##
# Validation Functions
##
- # TODO: How do we handle these errors?
- def no_duplicate_email_aliases
- if email_aliases.count != email_aliases.map(&:email).uniq.count
- errors.add(:email_aliases, "include a duplicate")
- end
- end
-
- def email_aliases_differ_from_email
+ def email_differs_from_email_aliases
+ return if email_aliases.last.errors.any?
if email_aliases.map(&:email).include?(email)
- errors.add(:email_aliases, "include the original email address")
+ errors.add(:email, "may not be the same as an alias")
end
end
diff --git a/users/app/views/emails/_email.html.haml b/users/app/views/emails/_email.html.haml
index f5eb2d0..3feb6f0 100644
--- a/users/app/views/emails/_email.html.haml
+++ b/users/app/views/emails/_email.html.haml
@@ -1,5 +1,6 @@
-%li.pull-right
- %code= email
- = link_to(user_email_alias_path(@user, email), :method => :delete) do
- %i.icon-remove
-.clearfix
+- if email.valid?
+ %li.pull-right
+ %code= email
+ = link_to(user_email_alias_path(@user, email), :method => :delete) do
+ %i.icon-remove
+ .clearfix
diff --git a/users/app/views/users/_email_aliases.html.haml b/users/app/views/users/_email_aliases.html.haml
index 646480e..121acc4 100644
--- a/users/app/views/users/_email_aliases.html.haml
+++ b/users/app/views/users/_email_aliases.html.haml
@@ -2,5 +2,5 @@
%ul.unstyled
=render @user.email_aliases
.clearfix
-= f.simple_fields_for :email_aliases, Email.new do |e|
+= f.simple_fields_for :email_aliases, @email_alias do |e|
= e.input :email, :placeholder => "alias@#{request.domain}"
diff --git a/users/test/unit/email_aliases_test.rb b/users/test/unit/email_aliases_test.rb
index 762aaea..f680ac6 100644
--- a/users/test/unit/email_aliases_test.rb
+++ b/users/test/unit/email_aliases_test.rb
@@ -22,7 +22,7 @@ class EmailAliasTest < ActiveSupport::TestCase
test "adding email alias directly" do
email_alias = "valid_alias@domain.net"
- @user.add_email_alias(email_alias)
+ @user.email_aliases.build :email => email_alias
assert @user.changed?
assert @user.save
assert_equal email_alias, @user.reload.email_aliases.first.to_s
@@ -30,10 +30,11 @@ class EmailAliasTest < ActiveSupport::TestCase
test "duplicated email aliases are invalid" do
email_alias = "valid_alias@domain.net"
- @user.add_email_alias(email_alias)
+ @user.email_aliases.build :email => email_alias
@user.save
# add again
- @user.add_email_alias(email_alias)
+ email_alias = @user.email_aliases.build :email => email_alias
+ assert !email_alias.valid?
assert @user.changed?
assert !@user.valid?
end
@@ -41,14 +42,14 @@ class EmailAliasTest < ActiveSupport::TestCase
test "email is invalid as email alias" do
email_alias = "valid_alias@domain.net"
@user.email = email_alias
- @user.add_email_alias(email_alias)
+ @user.email_aliases.build :email => email_alias
assert @user.changed?
assert !@user.valid?
end
test "find user by email alias" do
email_alias = "valid_alias@domain.net"
- @user.add_email_alias(email_alias)
+ @user.email_aliases.build :email => email_alias
assert @user.save
assert_equal @user, User.find_by_email_or_alias(email_alias)
assert_equal @user, User.find_by_email_alias(email_alias)
diff --git a/users/test/unit/email_test.rb b/users/test/unit/email_test.rb
index 1e216d6..5aa2b11 100644
--- a/users/test/unit/email_test.rb
+++ b/users/test/unit/email_test.rb
@@ -20,9 +20,9 @@ class EmailTest < ActiveSupport::TestCase
test "email aliases need to be unique" do
email_alias = "valid_alias@domain.net"
- @other_user.add_email_alias email_alias
+ @other_user.email_aliases.build :email => email_alias
@other_user.save
- @user.add_email_alias email_alias
+ @user.email_aliases.build :email => email_alias
assert @user.changed?
assert !@user.save
# TODO handle errors
@@ -32,7 +32,7 @@ class EmailTest < ActiveSupport::TestCase
email_alias = "valid_alias@domain.net"
@other_user.email = email_alias
@other_user.save
- @user.add_email_alias email_alias
+ @user.email_aliases.build :email => email_alias
assert @user.changed?
assert !@user.save
end