summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorazul <azul@riseup.net>2012-12-20 01:53:32 -0800
committerazul <azul@riseup.net>2012-12-20 01:53:32 -0800
commit69a6c34998328b2168053184e1e487b60e1cc536 (patch)
tree361b0be19535b0d7ddb3eaac7acdfcebc7a0e3a8
parente899e5c3f33acb3228fac295013d7cc8b6e4eb04 (diff)
parentdc827d0a80360aa245d4d724dc42bc47571faea6 (diff)
Merge pull request #5 from leapcode/feature/removing-email-aliases
Removing email aliases - proper error display
-rw-r--r--app/views/tabs/_nav.html.haml2
-rw-r--r--app/views/tabs/_tab.html.haml2
-rw-r--r--app/views/tabs/_tabs.html.haml4
-rw-r--r--users/app/assets/javascripts/users.js.coffee8
-rw-r--r--users/app/controllers/email_aliases_controller.rb25
-rw-r--r--users/app/controllers/users_controller.rb16
-rw-r--r--users/app/helpers/users_helper.rb1
-rw-r--r--users/app/models/email.rb8
-rw-r--r--users/app/models/local_email.rb21
-rw-r--r--users/app/models/user.rb28
-rw-r--r--users/app/views/emails/_email.html.haml10
-rw-r--r--users/app/views/users/_email_aliases.html.haml2
-rw-r--r--users/app/views/users/edit.html.haml24
-rw-r--r--users/config/locales/en.yml1
-rw-r--r--users/config/routes.rb2
-rw-r--r--users/test/functional/users_controller_test.rb4
-rw-r--r--users/test/unit/email_aliases_test.rb11
-rw-r--r--users/test/unit/email_test.rb8
18 files changed, 100 insertions, 77 deletions
diff --git a/app/views/tabs/_nav.html.haml b/app/views/tabs/_nav.html.haml
new file mode 100644
index 0000000..6974c06
--- /dev/null
+++ b/app/views/tabs/_nav.html.haml
@@ -0,0 +1,2 @@
+%li{:class => (:active if nav == @anchor)}
+ %a{:href => "##{nav}", 'data-toggle' => 'tab'}= t("tabs.#{nav}")
diff --git a/app/views/tabs/_tab.html.haml b/app/views/tabs/_tab.html.haml
new file mode 100644
index 0000000..adab666
--- /dev/null
+++ b/app/views/tabs/_tab.html.haml
@@ -0,0 +1,2 @@
+.tab-pane{:id => tab, :class => (:active if tab == @anchor)}
+ = content_for tab
diff --git a/app/views/tabs/_tabs.html.haml b/app/views/tabs/_tabs.html.haml
new file mode 100644
index 0000000..8b01dbd
--- /dev/null
+++ b/app/views/tabs/_tabs.html.haml
@@ -0,0 +1,4 @@
+%ul.nav.nav-tabs
+ = render :partial => '/tabs/nav', :collection => tabs
+.tab-content
+ = render :partial => '/tabs/tab', :collection => tabs
diff --git a/users/app/assets/javascripts/users.js.coffee b/users/app/assets/javascripts/users.js.coffee
index 0c1fb55..86bacee 100644
--- a/users/app/assets/javascripts/users.js.coffee
+++ b/users/app/assets/javascripts/users.js.coffee
@@ -24,7 +24,15 @@ srp.error = (message) ->
pollUsers = (query, process) ->
$.get( "/users.json", query: query).done(process)
+followLocationHash = ->
+ location = window.location.hash
+ if location
+ href_select = 'a[href="' + location + '"]'
+ link = $(href_select)
+ link.tab('show') if link
+
$(document).ready ->
+ followLocationHash()
$('#new_user').submit preventDefault
$('#new_user').submit srp.signup
$('#new_session').submit preventDefault
diff --git a/users/app/controllers/email_aliases_controller.rb b/users/app/controllers/email_aliases_controller.rb
index 751df85..3b0d5ac 100644
--- a/users/app/controllers/email_aliases_controller.rb
+++ b/users/app/controllers/email_aliases_controller.rb
@@ -4,29 +4,10 @@ class EmailAliasesController < ApplicationController
respond_to :html
- # get a list of email aliases for the given user?
- def index
- @aliases = @user.email_aliases
- respond_with @aliases
- end
-
- def create
- @alias = @user.add_email_alias(params[:email_alias])
- flash[:notice] = t(:email_alias_created_successfully) unless @alias.errors
- respond_with @alias, :location => edit_user_path(@user, :anchor => :email)
- end
-
- def update
- @alias = @user.get_email_alias(params[:id])
- @alias.set_email(params[:email_alias])
- flash[:notice] = t(:email_alias_updated_successfully) unless @alias.errors
- respond_with @alias, :location => edit_user_path(@user, :anchor => :email)
- end
-
def destroy
- @alias = @user.get_email_alias(params[:id])
- flash[:notice] = t(:email_alias_destroyed_successfully)
- @alias.destroy
+ @alias = @user.email_aliases.delete(params[:id])
+ @user.save
+ flash[:notice] = t(:email_alias_destroyed_successfully, :alias => @alias)
redirect_to edit_user_path(@user, :anchor => :email)
end
diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb
index 811e8e5..3d71c1a 100644
--- a/users/app/controllers/users_controller.rb
+++ b/users/app/controllers/users_controller.rb
@@ -3,6 +3,7 @@ class UsersController < ApplicationController
skip_before_filter :verify_authenticity_token, :only => [:create]
before_filter :fetch_user, :only => [:edit, :update, :destroy]
+ before_filter :set_anchor, :only => [:edit, :update]
before_filter :authorize_admin, :only => [:index]
respond_to :json, :html
@@ -27,16 +28,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[:error] = @user.errors.full_messages
end
- respond_with @user, :location => edit_user_path(@user)
+ respond_with @user, :location => edit_user_path(@user, :anchor => :email)
end
def destroy
@@ -50,4 +51,13 @@ class UsersController < ApplicationController
@user = User.find_by_param(params[:id])
access_denied unless admin? or (@user == current_user)
end
+
+ def set_anchor
+ @anchor = email_settings? ? :email : :account
+ end
+
+ def email_settings?
+ params[:user] &&
+ params[:user].keys.detect{|key| key.index('email')}
+ end
end
diff --git a/users/app/helpers/users_helper.rb b/users/app/helpers/users_helper.rb
index b017bca..45ca0e9 100644
--- a/users/app/helpers/users_helper.rb
+++ b/users/app/helpers/users_helper.rb
@@ -29,4 +29,5 @@ module UsersHelper
classes << (@user.new_record? ? 'new' : 'edit')
classes.compact
end
+
end
diff --git a/users/app/models/email.rb b/users/app/models/email.rb
index 4b01838..0745fda 100644
--- a/users/app/models/email.rb
+++ b/users/app/models/email.rb
@@ -14,4 +14,12 @@ class Email
def to_s
email
end
+
+ def ==(other)
+ other.is_a?(String) ? self.email == other : super
+ end
+
+ def to_param
+ email
+ end
end
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 10f358d..1999a28 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!
@@ -45,6 +43,7 @@ class User < CouchRest::Model::Base
view :by_login
view :by_created_at
view :by_email
+
view :by_email_alias,
:map => <<-EOJS
function(doc) {
@@ -56,6 +55,7 @@ class User < CouchRest::Model::Base
});
}
EOJS
+
view :by_email_or_alias,
:map => <<-EOJS
function(doc) {
@@ -70,6 +70,7 @@ class User < CouchRest::Model::Base
});
}
EOJS
+
end
class << self
@@ -114,33 +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.any? and 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 f182ed9..3feb6f0 100644
--- a/users/app/views/emails/_email.html.haml
+++ b/users/app/views/emails/_email.html.haml
@@ -1,4 +1,6 @@
-%li.pull-right
- %code= email
- %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/app/views/users/edit.html.haml b/users/app/views/users/edit.html.haml
index 92ab71b..820b80e 100644
--- a/users/app/views/users/edit.html.haml
+++ b/users/app/views/users/edit.html.haml
@@ -1,17 +1,11 @@
.span8.offset2
%h2=t :settings
- %ul.nav.nav-tabs
- %li.active
- %a{:href => '#account', 'data-toggle' => 'tab'}Account
- %li
- %a{:href => '#email', 'data-toggle' => 'tab'}Email
-
- .tab-content
- .tab-pane.active#account
- = user_form_with 'login_field', :legend => :change_login
- = user_form_with 'password_fields', :legend => :change_password
- = render 'cancel_account' if @user == current_user
- .tab-pane#email
- = user_form_with 'email_field', :legend => :set_email_address
- = user_form_with 'email_forward_field', :legend => :forward_email
- = user_form_with 'email_aliases', :legend => :add_email_alias
+ - content_for :account do
+ = user_form_with 'login_field', :legend => :change_login
+ = user_form_with 'password_fields', :legend => :change_password
+ = render 'cancel_account' if @user == current_user
+ - content_for :email do
+ = user_form_with 'email_field', :legend => :set_email_address
+ = user_form_with 'email_forward_field', :legend => :forward_email
+ = user_form_with 'email_aliases', :legend => :add_email_alias
+ = render 'tabs/tabs', :tabs => [:account, :email]
diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml
index d068e70..3c71e7e 100644
--- a/users/config/locales/en.yml
+++ b/users/config/locales/en.yml
@@ -15,6 +15,7 @@ en:
add_email_alias: "Add email alias"
user_updated_successfully: "Settings have been updated successfully."
user_created_successfully: "Successfully created your account."
+ email_alias_destroyed_successfully: "Successfully removed the alias '%{alias}'."
activemodel:
models:
diff --git a/users/config/routes.rb b/users/config/routes.rb
index 3c5fb73..8985502 100644
--- a/users/config/routes.rb
+++ b/users/config/routes.rb
@@ -11,7 +11,7 @@ Rails.application.routes.draw do
get "signup" => "users#new", :as => "signup"
resources :users do
- resources :email_aliases
+ resources :email_aliases, :only => [:destroy], :id => /.*/
end
end
diff --git a/users/test/functional/users_controller_test.rb b/users/test/functional/users_controller_test.rb
index ce17500..1fa1462 100644
--- a/users/test/functional/users_controller_test.rb
+++ b/users/test/functional/users_controller_test.rb
@@ -45,11 +45,12 @@ class UsersControllerTest < ActionController::TestCase
assert_equal user, assigns[:user]
end
- test "should process updated params" do
+ test "user can change settings" do
user = find_record User
user.expects(:attributes=).with(user.params)
user.expects(:changed?).returns(true)
user.expects(:save).returns(true)
+ user.stubs(:email_aliases).returns([])
login user
put :update, :user => user.params, :id => user.id, :format => :json
@@ -64,6 +65,7 @@ class UsersControllerTest < ActionController::TestCase
user.expects(:attributes=).with(user.params)
user.expects(:changed?).returns(true)
user.expects(:save).returns(true)
+ user.stubs(:email_aliases).returns([])
login :is_admin? => true
put :update, :user => user.params, :id => user.id, :format => :json
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..6f9beaa 100644
--- a/users/test/unit/email_test.rb
+++ b/users/test/unit/email_test.rb
@@ -14,15 +14,15 @@ class EmailTest < ActiveSupport::TestCase
teardown do
@user.destroy if @user.persisted? # just in case
- @other_user.destroy
+ @other_user.destroy if @other_user.persisted?
end
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