Skip to content
Browse files

Add email aliases for users

Emails are used to associate commits with users. The emails
are not verified and don't have to be valid email addresses. They
are assigned on a first come, first serve basis.

Notifications are sent when an email is added.
  • Loading branch information...
1 parent d41e404 commit 29cfd33d949d21d67f3892473c24d4f0a127dfe6 @jhollingsworth jhollingsworth committed
View
26 app/controllers/profiles/emails_controller.rb
@@ -0,0 +1,26 @@
+class Profiles::EmailsController < ApplicationController
+ layout "profile"
+
+ def index
+ @primary = current_user.email
+ @emails = current_user.emails
+ end
+
+ def create
+ @email = current_user.emails.new(params[:email])
+
+ flash[:alert] = @email.errors.full_messages.first unless @email.save
+
+ redirect_to profile_emails_url
+ end
+
+ def destroy
+ @email = current_user.emails.find(params[:id])
+ @email.destroy
+
+ respond_to do |format|
+ format.html { redirect_to profile_emails_url }
+ format.js { render nothing: true }
+ end
+ end
+end
View
15 app/helpers/commits_helper.rb
@@ -122,17 +122,18 @@ def get_old_file(project, commit, diff)
def commit_person_link(commit, options = {})
source_name = commit.send "#{options[:source]}_name".to_sym
source_email = commit.send "#{options[:source]}_email".to_sym
+
+ user = User.find_for_commit(source_email, source_name)
+ person_name = user.nil? ? source_name : user.name
+ person_email = user.nil? ? source_email : user.email
+
text = if options[:avatar]
- avatar = image_tag(avatar_icon(source_email, options[:size]), class: "avatar #{"s#{options[:size]}" if options[:size]}", width: options[:size], alt: "")
- %Q{#{avatar} <span class="commit-#{options[:source]}-name">#{source_name}</span>}
+ avatar = image_tag(avatar_icon(person_email, options[:size]), class: "avatar #{"s#{options[:size]}" if options[:size]}", width: options[:size], alt: "")
+ %Q{#{avatar} <span class="commit-#{options[:source]}-name">#{person_name}</span>}
else
- source_name
+ person_name
end
- # Prefer email match over name match
- user = User.where(email: source_email).first
- user ||= User.where(name: source_name).first
-
options = {
class: "commit-#{options[:source]}-link has_tooltip",
data: { :'original-title' => sanitize(source_email) }
View
6 app/mailers/emails/profile.rb
@@ -6,6 +6,12 @@ def new_user_email(user_id, password)
mail(to: @user.email, subject: subject("Account was created for you"))
end
+ def new_email_email(email_id)
+ @email = Email.find(email_id)
+ @user = @email.user
+ mail(to: @user.email, subject: subject("Email was added to your account"))
+ end
+
def new_ssh_key_email(key_id)
@key = Key.find(key_id)
@user = @key.user
View
33 app/models/email.rb
@@ -0,0 +1,33 @@
+# == Schema Information
+#
+# Table name: emails
+#
+# id :integer not null, primary key
+# user_id :integer not null
+# email :string not null
+# created_at :datetime not null
+class Email < ActiveRecord::Base
+ attr_accessible :email, :user_id
+
+ #
+ # Relations
+ #
+ belongs_to :user
+
+ #
+ # Validations
+ #
+ validates :user_id, presence: true
+ validates :email, presence: true, email: { strict_mode: true }, uniqueness: true
+ validate :unique_email, if: ->(email) { email.email_changed? }
+
+ before_validation :cleanup_email
+
+ def cleanup_email
+ self.email = self.email.downcase.strip
+ end
+
+ def unique_email
+ self.errors.add(:email, 'has already been taken') if User.exists?(email: self.email)
+ end
+end
View
13 app/models/user.rb
@@ -78,6 +78,7 @@ class User < ActiveRecord::Base
# Profile
has_many :keys, dependent: :destroy
+ has_many :emails, dependent: :destroy
# Groups
has_many :users_groups, dependent: :destroy
@@ -116,6 +117,7 @@ class User < ActiveRecord::Base
validates :notification_level, inclusion: { in: Notification.notification_levels }, presence: true
validate :namespace_uniq, if: ->(user) { user.username_changed? }
validate :avatar_type, if: ->(user) { user.avatar_changed? }
+ validate :unique_email, if: ->(user) { user.email_changed? }
validates :avatar, file_size: { maximum: 100.kilobytes.to_i }
before_validation :generate_password, on: :create
@@ -183,6 +185,13 @@ def find_for_database_authentication(warden_conditions)
where(conditions).first
end
end
+
+ def find_for_commit(email, name)
+ # Prefer email match over name match
+ User.where(email: email).first ||
+ User.joins(:emails).where(emails: { email: email }).first ||
+ User.where(name: name).first
+ end
def filter filter_name
case filter_name
@@ -250,6 +259,10 @@ def avatar_type
end
end
+ def unique_email
+ self.errors.add(:email, 'has already been taken') if Email.exists?(email: self.email)
+ end
+
# Groups user has access to
def authorized_groups
@authorized_groups ||= begin
View
5 app/observers/email_observer.rb
@@ -0,0 +1,5 @@
+class EmailObserver < BaseObserver
+ def after_create(email)
+ notification.new_email(email)
+ end
+end
View
4 app/services/git_push_service.rb
@@ -188,8 +188,6 @@ def is_default_branch? ref
end
def commit_user commit
- User.where(email: commit.author_email).first ||
- User.where(name: commit.author_name).first ||
- user
+ User.find_for_commit(commit.author_email, commit.author_name) || user
end
end
View
7 app/services/notification_service.rb
@@ -17,6 +17,13 @@ def new_key(key)
end
end
+ # Always notify user about email added to profile
+ def new_email(email)
+ if email.user
+ mailer.new_email_email(email.id)
+ end
+ end
+
# When create an issue we should send next emails:
#
# * issue assignee if their notification level is not Disabled
View
4 app/views/layouts/nav/_profile.html.haml
@@ -4,6 +4,10 @@
%i.icon-home
= nav_link(controller: :accounts) do
= link_to "Account", profile_account_path
+ = nav_link(controller: :emails) do
+ = link_to profile_emails_path do
+ Emails
+ %span.count= current_user.emails.count + 1
- unless current_user.ldap_user?
= nav_link(controller: :passwords) do
= link_to "Password", edit_profile_password_path
View
10 app/views/notify/new_email_email.html.haml
@@ -0,0 +1,10 @@
+%p
+ Hi #{@user.name}!
+%p
+ A new email was added to your account:
+%p
+ email:
+ %code= @email.email
+%p
+ If this email was added in error, you can remove it here:
+ = link_to "Emails", profile_emails_url
View
7 app/views/notify/new_email_email.text.erb
@@ -0,0 +1,7 @@
+Hi <%= @user.name %>!
+
+A new email was added to your account:
+
+email.................. <%= @email.email %>
+
+If this email was added in error, you can remove it here: <%= profile_emails_url %>
View
29 app/views/profiles/emails/index.html.haml
@@ -0,0 +1,29 @@
+%h3.page-title
+ My Email Addresses
+%p.light
+ Your
+ %b Primary Email
+ will be used for account notifications, avatar detection and web based operations, such as edits and merges. All email addresses will be used to identify your commits.
+
+.ui-box
+ .title
+ Emails (#{@emails.count + 1})
+ %ul.well-list#emails-table
+ %li
+ %strong= @primary
+ %span.label.label-success Primary Email
+ - @emails.each do |email|
+ %li
+ %strong= email.email
+ %span.cgray
+ added #{time_ago_with_tooltip(email.created_at)}
+ = link_to 'Remove', profile_email_path(email), data: { confirm: 'Are you sure?'}, method: :delete, class: 'btn btn-small btn-remove pull-right'
+
+%h3.page-title Add Email Address
+= form_for 'email', url: profile_emails_path, html: { class: 'form-horizontal' } do |f|
+ .form-group
+ = f.label :email, class: 'control-label'
+ .col-sm-10
+ = f.text_field :email, class: 'form-control'
+ .form-actions
+ = f.submit 'Add', class: 'btn btn-create'
View
1 config/routes.rb
@@ -124,6 +124,7 @@
end
end
resources :keys
+ resources :emails, only: [:index, :create, :destroy]
resources :groups, only: [:index] do
member do
delete :leave
View
13 db/migrate/20140209025651_create_emails.rb
@@ -0,0 +1,13 @@
+class CreateEmails < ActiveRecord::Migration
+ def change
+ create_table :emails do |t|
+ t.integer :user_id, null: false
+ t.string :email, null: false
+
+ t.timestamps
+ end
+
+ add_index :emails, :user_id
+ add_index :emails, :email, unique: true
+ end
+end
View
84 db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20140127170938) do
+ActiveRecord::Schema.define(version: 20140209025651) do
create_table "broadcast_messages", force: true do |t|
t.text "message", null: false
@@ -33,6 +33,16 @@
add_index "deploy_keys_projects", ["project_id"], name: "index_deploy_keys_projects_on_project_id", using: :btree
+ create_table "emails", force: true do |t|
+ t.integer "user_id", null: false
+ t.string "email", null: false
+ t.datetime "created_at"
+ t.datetime "updated_at"
+ end
+
+ add_index "emails", ["email"], name: "index_emails_on_email", unique: true, using: :btree
+ add_index "emails", ["user_id"], name: "index_emails_on_user_id", using: :btree
+
create_table "events", force: true do |t|
t.string "target_type"
t.integer "target_id"
@@ -66,8 +76,8 @@
t.integer "assignee_id"
t.integer "author_id"
t.integer "project_id"
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
t.integer "position", default: 0
t.string "branch_name"
t.text "description"
@@ -85,8 +95,8 @@
create_table "keys", force: true do |t|
t.integer "user_id"
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
t.text "key"
t.string "title"
t.string "type"
@@ -111,8 +121,8 @@
t.integer "author_id"
t.integer "assignee_id"
t.string "title"
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
t.integer "milestone_id"
t.string "state"
t.string "merge_status"
@@ -164,8 +174,8 @@
t.text "note"
t.string "noteable_type"
t.integer "author_id"
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
t.integer "project_id"
t.string "attachment"
t.string "line_code"
@@ -187,8 +197,8 @@
t.string "name"
t.string "path"
t.text "description"
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
t.integer "creator_id"
t.boolean "issues_enabled", default: true, null: false
t.boolean "wall_enabled", default: true, null: false
@@ -239,8 +249,8 @@
t.text "content", limit: 2147483647
t.integer "author_id", null: false
t.integer "project_id"
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
t.string "file_name"
t.datetime "expires_at"
t.boolean "private", default: true, null: false
@@ -262,42 +272,45 @@
t.datetime "created_at"
end
+ add_index "taggings", ["tag_id"], name: "index_taggings_on_tag_id", using: :btree
+ add_index "taggings", ["taggable_id", "taggable_type", "context"], name: "index_taggings_on_taggable_id_and_taggable_type_and_context", using: :btree
+
create_table "tags", force: true do |t|
t.string "name"
end
create_table "users", force: true do |t|
- t.string "email", default: "", null: false
- t.string "encrypted_password", limit: 128, default: "", null: false
+ t.string "email", default: "", null: false
+ t.string "encrypted_password", default: "", null: false
t.string "reset_password_token"
t.datetime "reset_password_sent_at"
t.datetime "remember_created_at"
- t.integer "sign_in_count", default: 0
+ t.integer "sign_in_count", default: 0
t.datetime "current_sign_in_at"
t.datetime "last_sign_in_at"
t.string "current_sign_in_ip"
t.string "last_sign_in_ip"
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
t.string "name"
- t.boolean "admin", default: false, null: false
- t.integer "projects_limit", default: 10
- t.string "skype", default: "", null: false
- t.string "linkedin", default: "", null: false
- t.string "twitter", default: "", null: false
+ t.boolean "admin", default: false, null: false
+ t.integer "projects_limit", default: 10
+ t.string "skype", default: "", null: false
+ t.string "linkedin", default: "", null: false
+ t.string "twitter", default: "", null: false
t.string "authentication_token"
- t.integer "theme_id", default: 1, null: false
+ t.integer "theme_id", default: 1, null: false
t.string "bio"
- t.integer "failed_attempts", default: 0
+ t.integer "failed_attempts", default: 0
t.datetime "locked_at"
t.string "extern_uid"
t.string "provider"
t.string "username"
- t.boolean "can_create_group", default: true, null: false
- t.boolean "can_create_team", default: true, null: false
+ t.boolean "can_create_group", default: true, null: false
+ t.boolean "can_create_team", default: true, null: false
t.string "state"
- t.integer "color_scheme_id", default: 1, null: false
- t.integer "notification_level", default: 1, null: false
+ t.integer "color_scheme_id", default: 1, null: false
+ t.integer "notification_level", default: 1, null: false
t.datetime "password_expires_at"
t.integer "created_by_id"
t.string "avatar"
@@ -305,14 +318,15 @@
t.datetime "confirmed_at"
t.datetime "confirmation_sent_at"
t.string "unconfirmed_email"
- t.boolean "hide_no_ssh_key", default: false
- t.string "website_url", default: "", null: false
+ t.boolean "hide_no_ssh_key", default: false
+ t.string "website_url", default: "", null: false
end
add_index "users", ["admin"], name: "index_users_on_admin", using: :btree
add_index "users", ["authentication_token"], name: "index_users_on_authentication_token", unique: true, using: :btree
add_index "users", ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true, using: :btree
add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree
+ add_index "users", ["extern_uid", "provider"], name: "index_users_on_extern_uid_and_provider", unique: true, using: :btree
add_index "users", ["name"], name: "index_users_on_name", using: :btree
add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree
add_index "users", ["username"], name: "index_users_on_username", using: :btree
@@ -331,8 +345,8 @@
create_table "users_projects", force: true do |t|
t.integer "user_id", null: false
t.integer "project_id", null: false
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
t.integer "project_access", default: 0, null: false
t.integer "notification_level", default: 3, null: false
end
@@ -344,8 +358,8 @@
create_table "web_hooks", force: true do |t|
t.string "url"
t.integer "project_id"
- t.datetime "created_at"
- t.datetime "updated_at"
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
t.string "type", default: "ProjectHook"
t.integer "service_id"
t.boolean "push_events", default: true, null: false
View
25 features/profile/emails.feature
@@ -0,0 +1,25 @@
+Feature: Profile Emails
+ Background:
+ Given I sign in as a user
+ And I visit profile emails page
+
+ Scenario: I should see emails
+ Then I should see my emails
+
+ Scenario: Add new email
+ Given I submit new email "my@email.com"
+ Then I should see new email "my@email.com"
+ And I should see my emails
+
+ Scenario: Add duplicate email
+ Given I submit duplicate email @user.email
+ Then I should not have @user.email added
+ And I should see my emails
+
+ Scenario: Remove email
+ Given I submit new email "my@email.com"
+ Then I should see new email "my@email.com"
+ And I should see my emails
+ Then I click link "Remove" for "my@email.com"
+ Then I should not see email "my@email.com"
+ And I should see my emails
View
14 features/project/commits/commits_user_lookup.feature
@@ -0,0 +1,14 @@
+Feature: Project Browse Commits User Lookup
+ Background:
+ Given I sign in as a user
+ And I own a project
+ And I have the user that authored the commits
+ And I visit my project's commits page
+
+ Scenario: I browse commit from list
+ Given I click on commit link
+ Then I see commit info
+
+ Scenario: I browse another commit from list
+ Given I click on another commit link
+ Then I see other commit info
View
48 features/steps/profile/profile_emails.rb
@@ -0,0 +1,48 @@
+class ProfileEmails < Spinach::FeatureSteps
+ include SharedAuthentication
+
+ Then 'I visit profile emails page' do
+ visit profile_emails_path
+ end
+
+ Then 'I should see my emails' do
+ page.should have_content(@user.email)
+ @user.emails.each do |email|
+ page.should have_content(email.email)
+ end
+ end
+
+ And 'I submit new email "my@email.com"' do
+ fill_in "email_email", with: "my@email.com"
+ click_button "Add"
+ end
+
+ Then 'I should see new email "my@email.com"' do
+ email = @user.emails.find_by(email: "my@email.com")
+ email.should_not be_nil
+ page.should have_content("my@email.com")
+ end
+
+ Then 'I should not see email "my@email.com"' do
+ email = @user.emails.find_by(email: "my@email.com")
+ email.should be_nil
+ page.should_not have_content("my@email.com")
+ end
+
+ Then 'I click link "Remove" for "my@email.com"' do
+ # there should only be one remove button at this time
+ click_link "Remove"
+ # force these to reload as they have been cached
+ @user.emails.reload
+ end
+
+ And 'I submit duplicate email @user.email' do
+ fill_in "email_email", with: @user.email
+ click_button "Add"
+ end
+
+ Then 'I should not have @user.email added' do
+ email = @user.emails.find_by(email: @user.email)
+ email.should be_nil
+ end
+end
View
35 features/steps/project/project_browse_commits_user_lookup.rb
@@ -0,0 +1,35 @@
+class ProjectBrowseCommitsUserLookup < Spinach::FeatureSteps
+ include SharedAuthentication
+ include SharedProject
+ include SharedPaths
+
+ Given 'I have the user that authored the commits' do
+ @user = create(:user, email: 'dmitriy.zaporozhets@gmail.com')
+ create(:email, { user: @user, email: 'dzaporozhets@sphereconsultinginc.com' })
+ end
+
+ Given 'I click on commit link' do
+ visit project_commit_path(@project, ValidCommit::ID)
+ end
+
+ Given 'I click on another commit link' do
+ visit project_commit_path(@project, ValidCommitWithAltEmail::ID)
+ end
+
+ Then 'I see commit info' do
+ page.should have_content ValidCommit::MESSAGE
+ check_author_link(ValidCommit::AUTHOR_EMAIL)
+ end
+
+ Then 'I see other commit info' do
+ page.should have_content ValidCommitWithAltEmail::MESSAGE
+ check_author_link(ValidCommitWithAltEmail::AUTHOR_EMAIL)
+ end
+
+ def check_author_link(email)
+ author_link = find('.commit-author-link')
+ author_link['href'].should == user_path(@user)
+ author_link['data-original-title'].should == email
+ find('.commit-author-name').text.should == @user.name
+ end
+end
View
2 features/support/env.rb
@@ -15,7 +15,7 @@
require 'sidekiq/testing/inline'
-%w(valid_commit big_commits select2_helper test_env).each do |f|
+%w(valid_commit valid_commit_with_alt_email big_commits select2_helper test_env).each do |f|
require Rails.root.join('spec', 'support', f)
end
View
13 spec/factories.rb
@@ -219,6 +219,19 @@
end
end
end
+
+ factory :email do
+ user
+ email do
+ Faker::Internet.email('alias')
+ end
+
+ factory :another_email do
+ email do
+ Faker::Internet.email('another.alias')
+ end
+ end
+ end
factory :milestone do
title
View
22 spec/mailers/notify_spec.rb
@@ -90,6 +90,28 @@
end
end
+ describe 'user added email' do
+ let(:email) { create(:email) }
+
+ subject { Notify.new_email_email(email.id) }
+
+ it 'is sent to the new user' do
+ should deliver_to email.user.email
+ end
+
+ it 'has the correct subject' do
+ should have_subject /^gitlab \| Email was added to your account$/i
+ end
+
+ it 'contains the new email address' do
+ should have_body_text /#{email.email}/
+ end
+
+ it 'includes a link to emails page' do
+ should have_body_text /#{profile_emails_path}/
+ end
+ end
+
context 'for a project' do
describe 'items that are assignable, the email' do
let(:assignee) { create(:user, email: 'assignee@example.com') }
View
17 spec/observers/email_observer_spec.rb
@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe EmailObserver do
+ let(:email) { create(:email) }
+
+ before { subject.stub(notification: double('NotificationService').as_null_object) }
+
+ subject { EmailObserver.instance }
+
+ describe '#after_create' do
+ it 'trigger notification to send emails' do
+ subject.should_receive(:notification)
+
+ subject.after_create(email)
+ end
+ end
+end
View
17 spec/routing/routing_spec.rb
@@ -183,6 +183,23 @@
end
end
+# emails GET /emails(.:format) emails#index
+# POST /keys(.:format) emails#create
+# DELETE /keys/:id(.:format) keys#destroy
+describe Profiles::EmailsController, "routing" do
+ it "to #index" do
+ get("/profile/emails").should route_to('profiles/emails#index')
+ end
+
+ it "to #create" do
+ post("/profile/emails").should route_to('profiles/emails#create')
+ end
+
+ it "to #destroy" do
+ delete("/profile/emails/1").should route_to('profiles/emails#destroy', id: '1')
+ end
+end
+
# profile_avatar DELETE /profile/avatar(.:format) profiles/avatars#destroy
describe Profiles::AvatarsController, "routing" do
it "to #destroy" do
View
13 spec/services/notification_service_spec.rb
@@ -16,6 +16,19 @@
end
end
+ describe 'Email' do
+ describe :new_email do
+ let(:email) { create(:email) }
+
+ it { notification.new_email(email).should be_true }
+
+ it 'should send email to email owner' do
+ Notify.should_receive(:new_email_email).with(email.id)
+ notification.new_email(email)
+ end
+ end
+ end
+
describe 'Notes' do
context 'issue note' do
let(:issue) { create(:issue, assignee: create(:user)) }
View
1 spec/support/valid_commit.rb
@@ -2,6 +2,7 @@ module ValidCommit
ID = "8470d70da67355c9c009e4401746b1d5410af2e3"
MESSAGE = "notes controller refactored"
AUTHOR_FULL_NAME = "Dmitriy Zaporozhets"
+ AUTHOR_EMAIL = "dmitriy.zaporozhets@gmail.com"
FILES = [".foreman", ".gitignore", ".rails_footnotes", ".rspec", ".travis.yml", "CHANGELOG", "Gemfile", "Gemfile.lock", "LICENSE", "Procfile", "Procfile.production", "README.md", "Rakefile", "VERSION", "app", "config.ru", "config", "db", "doc", "lib", "log", "public", "resque.sh", "script", "spec", "vendor"]
FILES_COUNT = 26
View
6 spec/support/valid_commit_with_alt_email.rb
@@ -0,0 +1,6 @@
+module ValidCommitWithAltEmail
+ ID = "1e689bfba39525ead225eaf611948cfbe8ac34cf"
+ MESSAGE = "fixed notes logic"
+ AUTHOR_FULL_NAME = "Dmitriy Zaporozhets"
+ AUTHOR_EMAIL = "dzaporozhets@sphereconsultinginc.com"
+end

0 comments on commit 29cfd33

Please sign in to comment.
Something went wrong with that request. Please try again.