Commit 205abf22 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'feature/email-aliases' into 'master'

Add email aliases for users
Email aliases 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.
This addresses http://feedback.gitlab.com/forums/176466-general/suggestions/3692666-ability-to-have-multiple-email-address.
parents d41e404e 29cfd33d
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
......@@ -122,17 +122,18 @@ module CommitsHelper
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) }
......
......@@ -6,6 +6,12 @@ module Emails
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
......
# == 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
\ No newline at end of file
......@@ -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
......@@ -184,6 +186,13 @@ class User < ActiveRecord::Base
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
when "admins"; self.admins
......@@ -250,6 +259,10 @@ class User < ActiveRecord::Base
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
......
class EmailObserver < BaseObserver
def after_create(email)
notification.new_email(email)
end
end
......@@ -188,8 +188,6 @@ class GitPushService
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
......@@ -17,6 +17,13 @@ class NotificationService
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
......
......@@ -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
......
%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
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 %>
%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'
\ No newline at end of file
......@@ -124,6 +124,7 @@ Gitlab::Application.routes.draw do
end
end
resources :keys
resources :emails, only: [:index, :create, :destroy]
resources :groups, only: [:index] do
member do
delete :leave
......
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
......@@ -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 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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,13 +272,16 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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 "encrypted_password", default: "", null: false
t.string "reset_password_token"
t.datetime "reset_password_sent_at"
t.datetime "remember_created_at"
......@@ -277,8 +290,8 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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
......@@ -313,6 +326,7 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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 @@ ActiveRecord::Schema.define(version: 20140127170938) do
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
......
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
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
\ No newline at end of file
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
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
......@@ -15,7 +15,7 @@ require 'spinach/capybara'
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
......
......@@ -220,6 +220,19 @@ FactoryGirl.define do
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
project
......
......@@ -90,6 +90,28 @@ describe Notify do
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') }
......
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
......@@ -183,6 +183,23 @@ describe Profiles::KeysController, "routing" do
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
......
......@@ -16,6 +16,19 @@ describe NotificationService do
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)) }
......
......@@ -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
......
module ValidCommitWithAltEmail
ID = "1e689bfba39525ead225eaf611948cfbe8ac34cf"
MESSAGE = "fixed notes logic"
AUTHOR_FULL_NAME = "Dmitriy Zaporozhets"
AUTHOR_EMAIL = "dzaporozhets@sphereconsultinginc.com"
end
\ No newline at end of file
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment