From 4dffda05477f74c90197948bb6f13bedc8b2a19f Mon Sep 17 00:00:00 2001 From: Azul Date: Sun, 9 Dec 2012 10:52:23 +0100 Subject: first steps towards email aliases * unit tests draft * controller draft --- users/app/controllers/email_alias_controller.rb | 36 ++++++++++++++++++ users/test/unit/email_alias_test.rb | 50 +++++++++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 users/app/controllers/email_alias_controller.rb create mode 100644 users/test/unit/email_alias_test.rb diff --git a/users/app/controllers/email_alias_controller.rb b/users/app/controllers/email_alias_controller.rb new file mode 100644 index 0000000..979c8ad --- /dev/null +++ b/users/app/controllers/email_alias_controller.rb @@ -0,0 +1,36 @@ +class EmailAliasesController < ApplicationController + + before_filter :fetch_user + + # 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 + 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 + end + + def destroy + @alias = @user.get_email_alias(params[:id]) + flash[:notice] = t(:email_alias_destroyed_successfully) + @alias.destroy + end + + protected + + def fetch_user + @user = User.find_by_param(params[:user_id]) + access_denied unless admin? or (@user == current_user) + end +end diff --git a/users/test/unit/email_alias_test.rb b/users/test/unit/email_alias_test.rb new file mode 100644 index 0000000..86cb10f --- /dev/null +++ b/users/test/unit/email_alias_test.rb @@ -0,0 +1,50 @@ +require 'test_helper' + +class EmailAliasTest < ActiveSupport::TestCase + + setup do + @attribs = User.valid_attributes_hash + User.find_by_login(@attribs[:login]).try(:destroy) + @user = User.new(@attribs) + end + + test "no email aliases set in the beginning" do + assert_equal [], @user.email_aliases + end + + test "adding email alias" do + email_alias = "valid_alias@domain.net" + @user.add_email_alias(email_alias) + assert_equal [email_alias], @user.email_aliases + end + + test "email aliases need to be unique" do + # todo build helper for this ... make_record(User) + email_alias = "valid_alias@domain.net" + attribs = User.valid_attributes_hash.merge(:login => "other_user") + User.find_by_login(attribs[:login]).try(:destroy) + other_user = User.new(attribs) + other_user.add_email_alias(email_alias) + @user.add_email_alias(email_alias) + # todo: how do we handle errors? Should email_alias become an ActiveModel? + assert_equal [], @user.email_aliases + end + + test "email aliases may not conflict with emails" do + # todo build helper for this ... make_record(User) + email_alias = "valid_alias@domain.net" + attribs = User.valid_attributes_hash.merge(:login => "other_user", :email => email_alias) + User.find_by_login(attribs[:login]).try(:destroy) + other_user = User.new(attribs) + @user.add_email_alias(email_alias) + # todo: how do we handle errors? Should email_alias become an ActiveModel? + assert_equal [], @user.email_aliases + end + + test "can retrieve user by email alias" do + email_alias = "valid_alias@domain.net" + @user.add_email_alias(email_alias) + assert_equal @user, User.find_by_email_alias(email_alias) + assert_equal @user, User.find_by_email(email_alias) + end +end -- cgit v1.2.3 From 9d99d340cfd3c55f21d38c1ba9f3f4574e40c46c Mon Sep 17 00:00:00 2001 From: Azul Date: Sun, 9 Dec 2012 13:19:54 +0100 Subject: basic form added to user settings, simple model created --- users/app/controllers/email_alias_controller.rb | 36 --------------------- users/app/controllers/email_aliases_controller.rb | 39 +++++++++++++++++++++++ users/app/helpers/email_aliases_helper.rb | 11 +++++++ users/app/models/email_alias.rb | 6 ++++ users/app/models/user.rb | 6 ++-- users/app/views/users/_email_aliases.html.haml | 10 ++++++ users/app/views/users/edit.html.haml | 1 + users/config/routes.rb | 4 ++- 8 files changed, 74 insertions(+), 39 deletions(-) delete mode 100644 users/app/controllers/email_alias_controller.rb create mode 100644 users/app/controllers/email_aliases_controller.rb create mode 100644 users/app/helpers/email_aliases_helper.rb create mode 100644 users/app/models/email_alias.rb create mode 100644 users/app/views/users/_email_aliases.html.haml diff --git a/users/app/controllers/email_alias_controller.rb b/users/app/controllers/email_alias_controller.rb deleted file mode 100644 index 979c8ad..0000000 --- a/users/app/controllers/email_alias_controller.rb +++ /dev/null @@ -1,36 +0,0 @@ -class EmailAliasesController < ApplicationController - - before_filter :fetch_user - - # 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 - 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 - end - - def destroy - @alias = @user.get_email_alias(params[:id]) - flash[:notice] = t(:email_alias_destroyed_successfully) - @alias.destroy - end - - protected - - def fetch_user - @user = User.find_by_param(params[:user_id]) - access_denied unless admin? or (@user == current_user) - end -end diff --git a/users/app/controllers/email_aliases_controller.rb b/users/app/controllers/email_aliases_controller.rb new file mode 100644 index 0000000..751df85 --- /dev/null +++ b/users/app/controllers/email_aliases_controller.rb @@ -0,0 +1,39 @@ +class EmailAliasesController < ApplicationController + + before_filter :fetch_user + + 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 + redirect_to edit_user_path(@user, :anchor => :email) + end + + protected + + def fetch_user + @user = User.find_by_param(params[:user_id]) + access_denied unless admin? or (@user == current_user) + end +end diff --git a/users/app/helpers/email_aliases_helper.rb b/users/app/helpers/email_aliases_helper.rb new file mode 100644 index 0000000..b56b068 --- /dev/null +++ b/users/app/helpers/email_aliases_helper.rb @@ -0,0 +1,11 @@ +module EmailAliasesHelper + + def email_alias_form(options = {}) + simple_form_for [@user, EmailAlias.new()], + :html => {:class => "form-horizontal email-alias form"}, + :validate => true do |f| + yield f + end + end + +end diff --git a/users/app/models/email_alias.rb b/users/app/models/email_alias.rb new file mode 100644 index 0000000..25e4b27 --- /dev/null +++ b/users/app/models/email_alias.rb @@ -0,0 +1,6 @@ +class EmailAlias + include CouchRest::Model::Embeddable + + property :email, String + timestamps! +end diff --git a/users/app/models/user.rb b/users/app/models/user.rb index ae271ce..81d5262 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -1,11 +1,13 @@ class User < CouchRest::Model::Base property :login, String, :accessible => true - property :email, String, :accessible => true - property :email_forward, String, :accessible => true property :password_verifier, String, :accessible => true property :password_salt, String, :accessible => true + property :email, String, :accessible => true + property :email_forward, String, :accessible => true + property :email_aliases, [EmailAlias] + validates :login, :password_salt, :password_verifier, :presence => true diff --git a/users/app/views/users/_email_aliases.html.haml b/users/app/views/users/_email_aliases.html.haml new file mode 100644 index 0000000..54eac0f --- /dev/null +++ b/users/app/views/users/_email_aliases.html.haml @@ -0,0 +1,10 @@ +%legend= t(:email_aliases) +%ul + - @user.email_aliases.each do |email| + %li= email += email_alias_form do |f| + =f.input :email, :placeholder => "alias@#{request.domain}" + .pull-right + %button{:type => :submit, :class => 'btn'} + %i.icon-plus + Add Email Alias diff --git a/users/app/views/users/edit.html.haml b/users/app/views/users/edit.html.haml index b33c19b..eb1bca4 100644 --- a/users/app/views/users/edit.html.haml +++ b/users/app/views/users/edit.html.haml @@ -14,3 +14,4 @@ .tab-pane#email = user_form_with 'email_field', :legend => :set_email_address = user_form_with 'email_forward_field', :legend => :forward_email + = render 'email_aliases' diff --git a/users/config/routes.rb b/users/config/routes.rb index 6de216f..3c5fb73 100644 --- a/users/config/routes.rb +++ b/users/config/routes.rb @@ -10,6 +10,8 @@ Rails.application.routes.draw do resources :sessions, :only => [:new, :create, :update, :destroy] get "signup" => "users#new", :as => "signup" - resources :users + resources :users do + resources :email_aliases + end end -- cgit v1.2.3 From e4c7f2fb8fa2833037508f1b88f802944855fd77 Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 10 Dec 2012 10:38:48 +0100 Subject: actually allow adding email aliases --- users/app/controllers/users_controller.rb | 3 ++- users/app/models/user.rb | 7 +++++++ users/app/views/users/_email_aliases.html.haml | 20 ++++++++++---------- users/app/views/users/edit.html.haml | 2 +- users/config/locales/en.yml | 1 + 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index 4921a4a..5055f4e 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -30,7 +30,8 @@ class UsersController < ApplicationController end def update - if @user.update_attributes(params[:user]) + @user.attributes = params[:user] + if @user.changed? and @user.save flash[:notice] = t(:user_updated_successfully) end respond_with @user, :location => edit_user_path(@user) diff --git a/users/app/models/user.rb b/users/app/models/user.rb index 81d5262..3ad69c7 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -75,6 +75,13 @@ class User < CouchRest::Model::Base APP_CONFIG['admins'].include? self.login end + def email_aliases_attributes=(attrs) + if attrs + email_alias = EmailAlias.new(attrs.values.first) + email_aliases << email_alias + end + end + protected def password password_verifier diff --git a/users/app/views/users/_email_aliases.html.haml b/users/app/views/users/_email_aliases.html.haml index 54eac0f..41d4f9e 100644 --- a/users/app/views/users/_email_aliases.html.haml +++ b/users/app/views/users/_email_aliases.html.haml @@ -1,10 +1,10 @@ -%legend= t(:email_aliases) -%ul - - @user.email_aliases.each do |email| - %li= email -= email_alias_form do |f| - =f.input :email, :placeholder => "alias@#{request.domain}" - .pull-right - %button{:type => :submit, :class => 'btn'} - %i.icon-plus - Add Email Alias +.span6 + %ul.unstyled + - @user.email_aliases.each do |email_alias| + %li.pull-right + %code= email_alias.email + %i.icon-remove + .clearfix +.clearfix += f.simple_fields_for :email_aliases, EmailAlias.new 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 eb1bca4..92ab71b 100644 --- a/users/app/views/users/edit.html.haml +++ b/users/app/views/users/edit.html.haml @@ -14,4 +14,4 @@ .tab-pane#email = user_form_with 'email_field', :legend => :set_email_address = user_form_with 'email_forward_field', :legend => :forward_email - = render 'email_aliases' + = user_form_with 'email_aliases', :legend => :add_email_alias diff --git a/users/config/locales/en.yml b/users/config/locales/en.yml index fe7e824..d068e70 100644 --- a/users/config/locales/en.yml +++ b/users/config/locales/en.yml @@ -12,6 +12,7 @@ en: set_email_address: "Set email address" forward_email: "Forward email" email_aliases: "Email aliases" + add_email_alias: "Add email alias" user_updated_successfully: "Settings have been updated successfully." user_created_successfully: "Successfully created your account." -- cgit v1.2.3 From 28b21959f39b0e28c450bba54b8696632a0187fa Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 10 Dec 2012 11:00:24 +0100 Subject: created generic Email class and use it with EmailAliases --- users/app/models/email.rb | 9 +++++ users/app/models/email_alias.rb | 6 ---- users/app/models/user.rb | 4 +-- users/app/views/emails/_email.html.haml | 4 +++ users/app/views/users/_email_aliases.html.haml | 8 ++--- users/test/unit/email_alias_test.rb | 50 -------------------------- users/test/unit/email_aliases_test.rb | 30 ++++++++++++++++ users/test/unit/email_test.rb | 33 +++++++++++++++++ 8 files changed, 80 insertions(+), 64 deletions(-) create mode 100644 users/app/models/email.rb delete mode 100644 users/app/models/email_alias.rb create mode 100644 users/app/views/emails/_email.html.haml delete mode 100644 users/test/unit/email_alias_test.rb create mode 100644 users/test/unit/email_aliases_test.rb create mode 100644 users/test/unit/email_test.rb diff --git a/users/app/models/email.rb b/users/app/models/email.rb new file mode 100644 index 0000000..0988d9f --- /dev/null +++ b/users/app/models/email.rb @@ -0,0 +1,9 @@ +class Email + include CouchRest::Model::Embeddable + + property :email, String + + def to_s + email + end +end diff --git a/users/app/models/email_alias.rb b/users/app/models/email_alias.rb deleted file mode 100644 index 25e4b27..0000000 --- a/users/app/models/email_alias.rb +++ /dev/null @@ -1,6 +0,0 @@ -class EmailAlias - include CouchRest::Model::Embeddable - - property :email, String - timestamps! -end diff --git a/users/app/models/user.rb b/users/app/models/user.rb index 3ad69c7..fcb211e 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -6,7 +6,7 @@ class User < CouchRest::Model::Base property :email, String, :accessible => true property :email_forward, String, :accessible => true - property :email_aliases, [EmailAlias] + property :email_aliases, [Email] validates :login, :password_salt, :password_verifier, :presence => true @@ -77,7 +77,7 @@ class User < CouchRest::Model::Base def email_aliases_attributes=(attrs) if attrs - email_alias = EmailAlias.new(attrs.values.first) + email_alias = Email.new(attrs.values.first) email_aliases << email_alias end end diff --git a/users/app/views/emails/_email.html.haml b/users/app/views/emails/_email.html.haml new file mode 100644 index 0000000..f182ed9 --- /dev/null +++ b/users/app/views/emails/_email.html.haml @@ -0,0 +1,4 @@ +%li.pull-right + %code= email + %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 41d4f9e..646480e 100644 --- a/users/app/views/users/_email_aliases.html.haml +++ b/users/app/views/users/_email_aliases.html.haml @@ -1,10 +1,6 @@ .span6 %ul.unstyled - - @user.email_aliases.each do |email_alias| - %li.pull-right - %code= email_alias.email - %i.icon-remove - .clearfix + =render @user.email_aliases .clearfix -= f.simple_fields_for :email_aliases, EmailAlias.new do |e| += f.simple_fields_for :email_aliases, Email.new do |e| = e.input :email, :placeholder => "alias@#{request.domain}" diff --git a/users/test/unit/email_alias_test.rb b/users/test/unit/email_alias_test.rb deleted file mode 100644 index 86cb10f..0000000 --- a/users/test/unit/email_alias_test.rb +++ /dev/null @@ -1,50 +0,0 @@ -require 'test_helper' - -class EmailAliasTest < ActiveSupport::TestCase - - setup do - @attribs = User.valid_attributes_hash - User.find_by_login(@attribs[:login]).try(:destroy) - @user = User.new(@attribs) - end - - test "no email aliases set in the beginning" do - assert_equal [], @user.email_aliases - end - - test "adding email alias" do - email_alias = "valid_alias@domain.net" - @user.add_email_alias(email_alias) - assert_equal [email_alias], @user.email_aliases - end - - test "email aliases need to be unique" do - # todo build helper for this ... make_record(User) - email_alias = "valid_alias@domain.net" - attribs = User.valid_attributes_hash.merge(:login => "other_user") - User.find_by_login(attribs[:login]).try(:destroy) - other_user = User.new(attribs) - other_user.add_email_alias(email_alias) - @user.add_email_alias(email_alias) - # todo: how do we handle errors? Should email_alias become an ActiveModel? - assert_equal [], @user.email_aliases - end - - test "email aliases may not conflict with emails" do - # todo build helper for this ... make_record(User) - email_alias = "valid_alias@domain.net" - attribs = User.valid_attributes_hash.merge(:login => "other_user", :email => email_alias) - User.find_by_login(attribs[:login]).try(:destroy) - other_user = User.new(attribs) - @user.add_email_alias(email_alias) - # todo: how do we handle errors? Should email_alias become an ActiveModel? - assert_equal [], @user.email_aliases - end - - test "can retrieve user by email alias" do - email_alias = "valid_alias@domain.net" - @user.add_email_alias(email_alias) - assert_equal @user, User.find_by_email_alias(email_alias) - assert_equal @user, User.find_by_email(email_alias) - end -end diff --git a/users/test/unit/email_aliases_test.rb b/users/test/unit/email_aliases_test.rb new file mode 100644 index 0000000..e737773 --- /dev/null +++ b/users/test/unit/email_aliases_test.rb @@ -0,0 +1,30 @@ +require 'test_helper' + +class EmailAliasTest < ActiveSupport::TestCase + + setup do + @attribs = User.valid_attributes_hash + User.find_by_login(@attribs[:login]).try(:destroy) + @user = User.new(@attribs) + end + + test "no email aliases set in the beginning" do + assert_equal [], @user.email_aliases + end + + test "adding email alias" do + email_alias = "valid_alias@domain.net" + @user.attributes = {:email_aliases_attributes => {"0" => {:email => email_alias}}} + assert @user.changed? + assert @user.save + assert_equal email_alias, @user.email_aliases.first.to_s + end + + test "can retrieve user by email alias" do + email_alias = "valid_alias@domain.net" + @user.attributes = {:email_aliases => [email_alias]} + @user.save + assert_equal @user, User.find_by_email_alias(email_alias) + assert_equal @user, User.find_by_email(email_alias) + end +end diff --git a/users/test/unit/email_test.rb b/users/test/unit/email_test.rb new file mode 100644 index 0000000..d421b77 --- /dev/null +++ b/users/test/unit/email_test.rb @@ -0,0 +1,33 @@ +require 'test_helper' + +class EmailAliasTest < ActiveSupport::TestCase + + setup do + @attribs = User.valid_attributes_hash + User.find_by_login(@attribs[:login]).try(:destroy) + @user = User.new(@attribs) + end + + test "email aliases need to be unique" do + # TODO build helper for this ... make_record(User) + email_alias = "valid_alias@domain.net" + attribs = User.valid_attributes_hash.merge(:login => "other_user") + User.find_by_login(attribs[:login]).try(:destroy) + other_user = User.new(attribs) + other_user.attributes = {:email_aliases => [email_alias]} + other_user.save + @user.attributes = {:email_aliases => [email_alias]} + assert !@user.valid? + # TODO handle errors + end + + test "email aliases may not conflict with emails" do + # TODO build helper for this ... make_record(User) + email_alias = "valid_alias@domain.net" + attribs = User.valid_attributes_hash.merge(:login => "other_user", :email => email_alias) + User.find_by_login(attribs[:login]).try(:destroy) + other_user = User.new(attribs) + @user.attributes = {:email_aliases => [email_alias]} + assert !@user.valid? + end +end -- cgit v1.2.3 From 2695bf598cfbe860b202ebc96a4243af391d4582 Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 10 Dec 2012 19:23:47 +0100 Subject: make sure client side validations also run after tabs were switched --- users/app/assets/javascripts/users.js.coffee | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/users/app/assets/javascripts/users.js.coffee b/users/app/assets/javascripts/users.js.coffee index 0595292..0c1fb55 100644 --- a/users/app/assets/javascripts/users.js.coffee +++ b/users/app/assets/javascripts/users.js.coffee @@ -32,3 +32,7 @@ $(document).ready -> $('.user.form.change_password').submit srp.update $('.user.form.change_password').submit preventDefault $('.user.typeahead').typeahead({source: pollUsers}); + $('a[data-toggle="tab"]').on('shown', -> + $(ClientSideValidations.selectors.forms).validate() + ) + -- cgit v1.2.3 From d9fa19106998bc6ac484494334dcf150bb6aa5d5 Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 10 Dec 2012 20:16:26 +0100 Subject: email format validations --- users/app/controllers/users_controller.rb | 2 ++ users/app/models/email.rb | 5 +++++ users/app/models/user.rb | 7 +++++++ 3 files changed, 14 insertions(+) diff --git a/users/app/controllers/users_controller.rb b/users/app/controllers/users_controller.rb index 5055f4e..811e8e5 100644 --- a/users/app/controllers/users_controller.rb +++ b/users/app/controllers/users_controller.rb @@ -33,6 +33,8 @@ class UsersController < ApplicationController @user.attributes = params[:user] 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) end diff --git a/users/app/models/email.rb b/users/app/models/email.rb index 0988d9f..7c88c51 100644 --- a/users/app/models/email.rb +++ b/users/app/models/email.rb @@ -3,6 +3,11 @@ class Email property :email, String + validates :email_forward, + :format => { :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/, :message => "needs to be a valid email address"} + + timestamps! + def to_s email end diff --git a/users/app/models/user.rb b/users/app/models/user.rb index fcb211e..a41554d 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -26,6 +26,13 @@ class User < CouchRest::Model::Base :confirmation => true, :format => { :with => /.{8}.*/, :message => "needs to be at least 8 characters long" } + # TODO: write a proper email validator to be used in the different places + validates :email, + :format => { :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/, :message => "needs to be a valid email address"} + + validates :email_forward, + :format => { :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/, :message => "needs to be a valid email address"} + timestamps! design do -- cgit v1.2.3 From 60e8fc3e1309d1c972a7695e3344e63f5d633a06 Mon Sep 17 00:00:00 2001 From: Azul Date: Wed, 12 Dec 2012 15:41:14 +0100 Subject: find users by email and aliases --- users/app/models/email.rb | 4 +--- users/app/models/user.rb | 30 ++++++++++++++++++++++++++++-- users/test/unit/email_aliases_test.rb | 9 +++++---- users/test/unit/user_test.rb | 9 +++++++++ 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/users/app/models/email.rb b/users/app/models/email.rb index 7c88c51..45101c1 100644 --- a/users/app/models/email.rb +++ b/users/app/models/email.rb @@ -3,11 +3,9 @@ class Email property :email, String - validates :email_forward, + validates :email, :format => { :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/, :message => "needs to be a valid email address"} - timestamps! - def to_s email end diff --git a/users/app/models/user.rb b/users/app/models/user.rb index a41554d..7d1691a 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -28,16 +28,42 @@ class User < CouchRest::Model::Base # TODO: write a proper email validator to be used in the different places validates :email, - :format => { :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/, :message => "needs to be a valid email address"} + :format => { :with => /\A(([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,}))?\Z/, :message => "needs to be a valid email address"} validates :email_forward, - :format => { :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/, :message => "needs to be a valid email address"} + :format => { :with => /\A(([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,}))?\Z/, :message => "needs to be a valid email address"} timestamps! design do view :by_login view :by_created_at + view :by_email + view :by_email_alias, + :map => <<-EOJS + function(doc) { + if (doc.type != 'User') { + return; + } + doc.email_aliases.forEach(function(alias){ + emit(alias.email, doc); + }); + } + EOJS + view :by_email_or_alias, + :map => <<-EOJS + function(doc) { + if (doc.type != 'User') { + return; + } + if (doc.email) { + emit(doc.email, doc); + } + doc.email_aliases.forEach(function(alias){ + emit(alias.email, doc); + }); + } + EOJS end class << self diff --git a/users/test/unit/email_aliases_test.rb b/users/test/unit/email_aliases_test.rb index e737773..5ded5bb 100644 --- a/users/test/unit/email_aliases_test.rb +++ b/users/test/unit/email_aliases_test.rb @@ -20,11 +20,12 @@ class EmailAliasTest < ActiveSupport::TestCase assert_equal email_alias, @user.email_aliases.first.to_s end - test "can retrieve user by email alias" do + test "find user by email alias" do email_alias = "valid_alias@domain.net" - @user.attributes = {:email_aliases => [email_alias]} - @user.save + @user.attributes = {:email_aliases_attributes => {"0" => {: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) - assert_equal @user, User.find_by_email(email_alias) + assert_nil User.find_by_email(email_alias) end end diff --git a/users/test/unit/user_test.rb b/users/test/unit/user_test.rb index cce11c2..29f6a89 100644 --- a/users/test/unit/user_test.rb +++ b/users/test/unit/user_test.rb @@ -49,4 +49,13 @@ class UserTest < ActiveSupport::TestCase assert_equal client_rnd, srp_session.aa end + test "find user by email" do + email = "tryto@find.me" + @user.email = email + @user.save + assert_equal @user, User.find_by_email(email) + assert_equal @user, User.find_by_email_or_alias(email) + assert_nil User.find_by_email_alias(email) + end + end -- cgit v1.2.3 From d7890d7c8af6691df2817a9b6654acf9377847bd Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 13 Dec 2012 11:49:26 +0100 Subject: LocalEmail added - will validate uniqueness amongst emails and aliases --- users/app/models/local_email.rb | 11 +++++++++++ users/app/models/user.rb | 2 +- users/test/unit/email_test.rb | 36 +++++++++++++++++++++--------------- 3 files changed, 33 insertions(+), 16 deletions(-) create mode 100644 users/app/models/local_email.rb diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb new file mode 100644 index 0000000..5762a33 --- /dev/null +++ b/users/app/models/local_email.rb @@ -0,0 +1,11 @@ +class LocalEmail < Email + + validate :unique_on_server + + 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") + end + end +end diff --git a/users/app/models/user.rb b/users/app/models/user.rb index 7d1691a..e5e388b 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -6,7 +6,7 @@ class User < CouchRest::Model::Base property :email, String, :accessible => true property :email_forward, String, :accessible => true - property :email_aliases, [Email] + property :email_aliases, [LocalEmail] validates :login, :password_salt, :password_verifier, :presence => true diff --git a/users/test/unit/email_test.rb b/users/test/unit/email_test.rb index d421b77..cba88a2 100644 --- a/users/test/unit/email_test.rb +++ b/users/test/unit/email_test.rb @@ -1,33 +1,39 @@ require 'test_helper' -class EmailAliasTest < ActiveSupport::TestCase +class EmailTest < ActiveSupport::TestCase setup do + # TODO build helper for this ... make_record(User) @attribs = User.valid_attributes_hash User.find_by_login(@attribs[:login]).try(:destroy) @user = User.new(@attribs) + @attribs.merge!(:login => "other_user") + User.find_by_login(@attribs[:login]).try(:destroy) + @other_user = User.create(@attribs) + end + + teardown do + @user.destroy if @user.persisted? # just in case + @other_user.destroy end + test "email aliases need to be unique" do - # TODO build helper for this ... make_record(User) email_alias = "valid_alias@domain.net" - attribs = User.valid_attributes_hash.merge(:login => "other_user") - User.find_by_login(attribs[:login]).try(:destroy) - other_user = User.new(attribs) - other_user.attributes = {:email_aliases => [email_alias]} - other_user.save - @user.attributes = {:email_aliases => [email_alias]} - assert !@user.valid? + @other_user.attributes = {:email_aliases_attributes => {"0" => {:email => email_alias}}} + @other_user.save + @user.attributes = {:email_aliases_attributes => {"0" => {:email => email_alias}}} + assert @user.changed? + assert !@user.save # TODO handle errors end test "email aliases may not conflict with emails" do - # TODO build helper for this ... make_record(User) email_alias = "valid_alias@domain.net" - attribs = User.valid_attributes_hash.merge(:login => "other_user", :email => email_alias) - User.find_by_login(attribs[:login]).try(:destroy) - other_user = User.new(attribs) - @user.attributes = {:email_aliases => [email_alias]} - assert !@user.valid? + @other_user.email = email_alias + @other_user.save + @user.attributes = {:email_aliases_attributes => {"0" => {:email => email_alias}}} + assert @user.changed? + assert !@user.save end end -- cgit v1.2.3 From 6ff4bd80e0394260c8cd300cfb051451fc7e358f Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 13 Dec 2012 15:47:08 +0100 Subject: ensure users do not have duplicate email aliases nor aliases that are the same as the original email for that matter --- users/app/models/user.rb | 21 +++++++++++++++++++++ users/test/unit/email_aliases_test.rb | 22 ++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/users/app/models/user.rb b/users/app/models/user.rb index e5e388b..0eb244c 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -33,6 +33,10 @@ 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 + timestamps! design do @@ -115,6 +119,23 @@ class User < CouchRest::Model::Base end 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 + if email_aliases.map(&:email).include?(email) + errors.add(:email_aliases, "include the original email address") + end + end + protected def password password_verifier diff --git a/users/test/unit/email_aliases_test.rb b/users/test/unit/email_aliases_test.rb index 5ded5bb..cec0c0b 100644 --- a/users/test/unit/email_aliases_test.rb +++ b/users/test/unit/email_aliases_test.rb @@ -20,6 +20,28 @@ class EmailAliasTest < ActiveSupport::TestCase assert_equal email_alias, @user.email_aliases.first.to_s end + test "duplicated email aliases are invalid" do + email_alias = "valid_alias@domain.net" + email_aliases = { + "0" => {:email => email_alias}, + "1" => {:email => email_alias} + } + @user.attributes = {:email_aliases_attributes => email_aliases} + @user.save + # add some more + @user.attributes = {:email_aliases_attributes => email_aliases} + assert @user.changed? + assert !@user.valid? + end + + test "email is invalid as email alias" do + email_alias = "valid_alias@domain.net" + email_aliases = { "0" => {:email => email_alias} } + @user.attributes = {:email_aliases_attributes => email_aliases, :email => email_alias} + assert @user.changed? + assert !@user.valid? + end + test "find user by email alias" do email_alias = "valid_alias@domain.net" @user.attributes = {:email_aliases_attributes => {"0" => {:email => email_alias}}} -- cgit v1.2.3 From 9762a37ab932ee1a94e973977520c7f4673d78b1 Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 13 Dec 2012 16:01:59 +0100 Subject: refactor: allow adding email aliases directly --- users/app/models/user.rb | 9 ++++++++- users/test/unit/email_aliases_test.rb | 26 +++++++++++++++----------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/users/app/models/user.rb b/users/app/models/user.rb index 0eb244c..7c9002b 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -112,9 +112,16 @@ class User < CouchRest::Model::Base APP_CONFIG['admins'].include? self.login end + def add_email(email) + email = LocalEmail.new({:email => 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 - email_alias = Email.new(attrs.values.first) + email_alias = LocalEmail.new(attrs.values.first) email_aliases << email_alias end end diff --git a/users/test/unit/email_aliases_test.rb b/users/test/unit/email_aliases_test.rb index cec0c0b..3c731e1 100644 --- a/users/test/unit/email_aliases_test.rb +++ b/users/test/unit/email_aliases_test.rb @@ -12,7 +12,7 @@ class EmailAliasTest < ActiveSupport::TestCase assert_equal [], @user.email_aliases end - test "adding email alias" do + test "adding email alias through params" do email_alias = "valid_alias@domain.net" @user.attributes = {:email_aliases_attributes => {"0" => {:email => email_alias}}} assert @user.changed? @@ -20,31 +20,35 @@ class EmailAliasTest < ActiveSupport::TestCase assert_equal email_alias, @user.email_aliases.first.to_s end + test "adding email alias directly" do + email_alias = "valid_alias@domain.net" + @user.add_email(email_alias) + assert @user.changed? + assert @user.save + assert_equal email_alias, @user.reload.email_aliases.first.to_s + end + test "duplicated email aliases are invalid" do email_alias = "valid_alias@domain.net" - email_aliases = { - "0" => {:email => email_alias}, - "1" => {:email => email_alias} - } - @user.attributes = {:email_aliases_attributes => email_aliases} + @user.add_email(email_alias) @user.save - # add some more - @user.attributes = {:email_aliases_attributes => email_aliases} + # add again + @user.add_email(email_alias) assert @user.changed? assert !@user.valid? end test "email is invalid as email alias" do email_alias = "valid_alias@domain.net" - email_aliases = { "0" => {:email => email_alias} } - @user.attributes = {:email_aliases_attributes => email_aliases, :email => email_alias} + @user.email = email_alias + @user.add_email(email_alias) assert @user.changed? assert !@user.valid? end test "find user by email alias" do email_alias = "valid_alias@domain.net" - @user.attributes = {:email_aliases_attributes => {"0" => {:email => email_alias}}} + @user.add_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) -- cgit v1.2.3 From 577c3d8149acbd483120847b994582268f93c0b3 Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 13 Dec 2012 16:07:31 +0100 Subject: refactor: Email constructor now takes string or hash This allows us to reuse add_email from email_aliases_attributes= --- users/app/models/email.rb | 5 +++++ users/app/models/user.rb | 7 +++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/users/app/models/email.rb b/users/app/models/email.rb index 45101c1..4b01838 100644 --- a/users/app/models/email.rb +++ b/users/app/models/email.rb @@ -6,6 +6,11 @@ class Email validates :email, :format => { :with => /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\Z/, :message => "needs to be a valid email address"} + def initialize(attributes = nil, &block) + attributes = {:email => attributes} if attributes.is_a? String + super(attributes, &block) + end + def to_s email end diff --git a/users/app/models/user.rb b/users/app/models/user.rb index 7c9002b..b531dfd 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -113,16 +113,15 @@ class User < CouchRest::Model::Base end def add_email(email) - email = LocalEmail.new({:email => email}) unless email.is_a? 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 - email_alias = LocalEmail.new(attrs.values.first) - email_aliases << email_alias + if attrs && attrs.values.first + add_email attrs.values.first end end -- cgit v1.2.3 From aed03c04270bc9b7d48048ffb2f24fecc993c7ac Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 13 Dec 2012 16:13:32 +0100 Subject: refactor: changed add_email to add_email_alias that's what it does. Changed all tests to use it instead of the attributes method --- users/app/models/user.rb | 4 ++-- users/test/unit/email_aliases_test.rb | 10 +++++----- users/test/unit/email_test.rb | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/users/app/models/user.rb b/users/app/models/user.rb index b531dfd..4fd0039 100644 --- a/users/app/models/user.rb +++ b/users/app/models/user.rb @@ -112,7 +112,7 @@ class User < CouchRest::Model::Base APP_CONFIG['admins'].include? self.login end - def add_email(email) + def add_email_alias(email) email = LocalEmail.new(email) unless email.is_a? Email email_aliases << email end @@ -121,7 +121,7 @@ class User < CouchRest::Model::Base # All the ui needs for now. def email_aliases_attributes=(attrs) if attrs && attrs.values.first - add_email attrs.values.first + add_email_alias attrs.values.first end end diff --git a/users/test/unit/email_aliases_test.rb b/users/test/unit/email_aliases_test.rb index 3c731e1..762aaea 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(email_alias) + @user.add_email_alias(email_alias) assert @user.changed? assert @user.save assert_equal email_alias, @user.reload.email_aliases.first.to_s @@ -30,10 +30,10 @@ class EmailAliasTest < ActiveSupport::TestCase test "duplicated email aliases are invalid" do email_alias = "valid_alias@domain.net" - @user.add_email(email_alias) + @user.add_email_alias(email_alias) @user.save # add again - @user.add_email(email_alias) + @user.add_email_alias(email_alias) assert @user.changed? assert !@user.valid? end @@ -41,14 +41,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(email_alias) + @user.add_email_alias(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(email_alias) + @user.add_email_alias(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 cba88a2..1e216d6 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.attributes = {:email_aliases_attributes => {"0" => {:email => email_alias}}} + @other_user.add_email_alias email_alias @other_user.save - @user.attributes = {:email_aliases_attributes => {"0" => {:email => email_alias}}} + @user.add_email_alias 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.attributes = {:email_aliases_attributes => {"0" => {:email => email_alias}}} + @user.add_email_alias email_alias assert @user.changed? assert !@user.save end -- cgit v1.2.3 From feee17e3a9970ac04f5a789255fbf4c3e8e89eb8 Mon Sep 17 00:00:00 2001 From: Azul Date: Thu, 13 Dec 2012 17:15:14 +0100 Subject: use the same partial for Email and LocalEmail --- users/app/models/local_email.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/users/app/models/local_email.rb b/users/app/models/local_email.rb index 5762a33..7cca4f4 100644 --- a/users/app/models/local_email.rb +++ b/users/app/models/local_email.rb @@ -2,6 +2,10 @@ class LocalEmail < Email validate :unique_on_server + def to_partial_path + "emails/email" + end + def unique_on_server has_email = User.find_by_email_or_alias(email) if has_email && has_email != self.base_doc -- cgit v1.2.3