Merge branch 'mk-add-opt-in-email-checkbox' into 'master'

Add opt-in "I'd like to receive updates via email" checkbox

Closes #2447

See merge request !2623

app/controllers/registrations_controller.rb

 class RegistrationsController < Devise::RegistrationsController
   include Recaptcha::Verify
+  prepend EE::RegistrationsController
 
   def new
     redirect_to(new_user_session_path)

app/services/system_hooks_service.rb

 class SystemHooksService
+  prepend EE::SystemHooksService
+
   def execute_hooks_for(model, event)
     execute_hooks(build_event_data(model, event))
   end
@@ -35,12 +37,7 @@ class SystemHooksService
         data[:old_path_with_namespace] = model.old_path_with_namespace
       end
     when User
-      data.merge!({
-        name: model.name,
-        email: model.email,
-        user_id: model.id,
-        username: model.username
-      })
+      data.merge!(user_data(model))
     when ProjectMember
       data.merge!(project_member_data(model))
     when Group
@@ -116,4 +113,13 @@ class SystemHooksService
       group_access: model.human_access
     }
   end
+
+  def user_data(model)
+    {
+      name: model.name,
+      email: model.email,
+      user_id: model.id,
+      username: model.username
+    }
+  end
 end

app/services/users/build_service.rb

 module Users
   class BuildService < BaseService
+    prepend ::EE::Users::BuildService
+
     def initialize(current_user, params = {})
       @current_user = current_user
       @params = params.dup

app/views/devise/shared/_signup_box.html.haml

@@ -22,6 +22,7 @@
         = f.label :password
         = f.password_field :password, class: "form-control bottom", required: true, pattern: ".{#{@minimum_password_length},}", title: "Minimum length is #{@minimum_password_length} characters."
         %p.gl-field-hint Minimum length is #{@minimum_password_length} characters
+      = render 'devise/shared/ee/email_opted_in', f: f
       %div
       - if Gitlab::Recaptcha.enabled?
         = recaptcha_tags

db/migrate/20170808005254_add_email_opted_in_fields_to_users.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddEmailOptedInFieldsToUsers < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :users, :email_opted_in, :boolean, null: true
+    add_column :users, :email_opted_in_ip, :string, null: true
+    add_column :users, :email_opted_in_source_id, :integer, null: true
+    add_column :users, :email_opted_in_at, :datetime_with_timezone, null: true
+  end
+end

db/schema.rb

@@ -1904,6 +1904,10 @@ ActiveRecord::Schema.define(version: 20170808163512) do
     t.string "rss_token"
     t.boolean "external_email", default: false, null: false
     t.string "email_provider"
+    t.boolean "email_opted_in"
+    t.string "email_opted_in_ip"
+    t.integer "email_opted_in_source_id"
+    t.datetime_with_timezone "email_opted_in_at"
   end
 
   add_index "users", ["admin"], name: "index_users_on_admin", using: :btree

ee/app/controllers/ee/registrations_controller.rb

+module EE
+  module RegistrationsController
+    extend ActiveSupport::Concern
+
+    private
+
+    def sign_up_params
+      clean_params = params.require(:user).permit(:username, :email, :email_confirmation, :name, :password, :email_opted_in)
+
+      if clean_params[:email_opted_in] == '1'
+        clean_params[:email_opted_in_ip] = request.remote_ip
+        clean_params[:email_opted_in_source_id] = User::EMAIL_OPT_IN_SOURCE_ID_GITLAB_COM
+        clean_params[:email_opted_in_at] = Time.zone.now
+      end
+
+      clean_params
+    end
+  end
+end

ee/app/models/ee/user.rb

@@ -8,6 +8,8 @@ module EE
     include AuditorUserHelper
 
     included do
+      EMAIL_OPT_IN_SOURCE_ID_GITLAB_COM = 1
+
       # We aren't using the `auditor?` method for the `if` condition here
       # because `auditor?` returns `false` when the `auditor` column is `true`
       # and the auditor add-on absent. We want to run this validation
@@ -91,5 +93,9 @@ module EE
       return if ::Gitlab::Geo.secondary?
       super
     end
+
+    def email_opted_in_source
+      email_opted_in_source_id == EMAIL_OPT_IN_SOURCE_ID_GITLAB_COM ? 'GitLab.com' : ''
+    end
   end
 end

ee/app/services/ee/system_hooks_service.rb

+module EE
+  module SystemHooksService
+    # override
+
+    private
+
+    def user_data(model)
+      data = super
+      data.merge!(email_opted_in_data(model)) if ::Gitlab.com?
+      data
+    end
+
+    def email_opted_in_data(model)
+      {
+        email_opted_in: model.email_opted_in,
+        email_opted_in_ip: model.email_opted_in_ip,
+        email_opted_in_source: model.email_opted_in_source,
+        email_opted_in_at: model.email_opted_in_at
+      }
+    end
+  end
+end

ee/app/services/ee/users/build_service.rb

+module EE
+  module Users
+    module BuildService
+      private
+
+      def signup_params
+        super + email_opted_in_params
+      end
+
+      def email_opted_in_params
+        [
+          :email_opted_in,
+          :email_opted_in_ip,
+          :email_opted_in_source_id,
+          :email_opted_in_at
+        ]
+      end
+    end
+  end
+end

ee/app/views/devise/shared/ee/_email_opted_in.html.haml

+- if Gitlab.com?
+  .form-group
+    = f.check_box :email_opted_in
+    = f.label :email_opted_in, "I'd like to receive updates via email about GitLab."

spec/ee/spec/controllers/ee/registrations_controller_spec.rb

+require 'spec_helper'
+
+describe RegistrationsController do
+  describe '#create' do
+    context 'when the user opted-in' do
+      let(:user_params) { { user: attributes_for(:user, email_opted_in: '1') } }
+
+      it 'sets the rest of the email_opted_in fields' do
+        post :create, user_params
+        user = User.find_by_username!(user_params[:user][:username])
+        expect(user.email_opted_in).to be_truthy
+        expect(user.email_opted_in_ip).to be_present
+        expect(user.email_opted_in_source).to eq('GitLab.com')
+        expect(user.email_opted_in_at).not_to be_nil
+      end
+    end
+
+    context 'when the user opted-out' do
+      let(:user_params) { { user: attributes_for(:user, email_opted_in: '0') } }
+
+      it 'does not set the rest of the email_opted_in fields' do
+        post :create, user_params
+        user = User.find_by_username!(user_params[:user][:username])
+        expect(user.email_opted_in).to be_falsey
+        expect(user.email_opted_in_ip).to be_blank
+        expect(user.email_opted_in_source).to be_blank
+        expect(user.email_opted_in_at).to be_nil
+      end
+    end
+  end
+end

spec/ee/spec/features/signup_spec.rb

+require 'spec_helper'
+
+feature 'Signup on EE' do
+  let(:user_attrs) { attributes_for(:user) }
+
+  context 'for Gitlab.com' do
+    before do
+      expect(Gitlab).to receive(:com?).and_return(true).at_least(:once)
+    end
+
+    context 'when the user checks the opt-in to email updates box' do
+      it 'creates the user and sets the email_opted_in field truthy' do
+        visit root_path
+
+        fill_in 'new_user_name',                with: user_attrs[:name]
+        fill_in 'new_user_username',            with: user_attrs[:username]
+        fill_in 'new_user_email',               with: user_attrs[:email]
+        fill_in 'new_user_email_confirmation',  with: user_attrs[:email]
+        fill_in 'new_user_password',            with: user_attrs[:password]
+        check   'new_user_email_opted_in'
+        click_button "Register"
+
+        user = User.find_by_username!(user_attrs[:username])
+        expect(user.email_opted_in).to be_truthy
+        expect(user.email_opted_in_ip).to be_present
+        expect(user.email_opted_in_source).to eq('GitLab.com')
+        expect(user.email_opted_in_at).not_to be_nil
+      end
+    end
+
+    context 'when the user does not check the opt-in to email updates box' do
+      it 'creates the user and sets the email_opted_in field falsey' do
+        visit root_path
+
+        fill_in 'new_user_name',                with: user_attrs[:name]
+        fill_in 'new_user_username',            with: user_attrs[:username]
+        fill_in 'new_user_email',               with: user_attrs[:email]
+        fill_in 'new_user_email_confirmation',  with: user_attrs[:email]
+        fill_in 'new_user_password',            with: user_attrs[:password]
+        click_button "Register"
+
+        user = User.find_by_username!(user_attrs[:username])
+        expect(user.email_opted_in).to be_falsey
+        expect(user.email_opted_in_ip).to be_blank
+        expect(user.email_opted_in_source).to be_blank
+        expect(user.email_opted_in_at).to be_nil
+      end
+    end
+  end
+
+  context 'not for Gitlab.com' do
+    before do
+      expect(Gitlab).to receive(:com?).and_return(false).at_least(:once)
+    end
+
+    it 'does not have a opt-in checkbox, it creates the user and sets email_opted_in to falsey' do
+      visit root_path
+
+      expect(page).not_to have_selector("[name='new_user_email_opted_in']")
+
+      fill_in 'new_user_name',                with: user_attrs[:name]
+      fill_in 'new_user_username',            with: user_attrs[:username]
+      fill_in 'new_user_email',               with: user_attrs[:email]
+      fill_in 'new_user_email_confirmation',  with: user_attrs[:email]
+      fill_in 'new_user_password',            with: user_attrs[:password]
+      click_button "Register"
+
+      user = User.find_by_username!(user_attrs[:username])
+      expect(user.email_opted_in).to be_falsey
+      expect(user.email_opted_in_ip).to be_blank
+      expect(user.email_opted_in_source).to be_blank
+      expect(user.email_opted_in_at).to be_nil
+    end
+  end
+end

spec/ee/spec/models/ee/user_spec.rb

@@ -105,4 +105,30 @@ describe EE::User do
       expect { subject.remember_me! }.not_to change(subject, :remember_created_at)
     end
   end
+
+  describe '#email_opted_in_source' do
+    context 'for GitLab.com' do
+      let(:user) { build(:user, email_opted_in_source_id: 1) }
+
+      it 'returns GitLab.com' do
+        expect(user.email_opted_in_source).to eq('GitLab.com')
+      end
+    end
+
+    context 'for nil source id' do
+      let(:user) { build(:user, email_opted_in_source_id: nil) }
+
+      it 'returns blank' do
+        expect(user.email_opted_in_source).to be_blank
+      end
+    end
+
+    context 'for non-existent source id' do
+      let(:user) { build(:user, email_opted_in_source_id: 2) }
+
+      it 'returns blank' do
+        expect(user.email_opted_in_source).to be_blank
+      end
+    end
+  end
 end

spec/ee/spec/services/ee/system_hooks_service_spec.rb

+require 'spec_helper'
+
+describe EE::SystemHooksService do
+  let(:user) { create(:user) }
+
+  context 'event data' do
+    context 'for GitLab.com' do
+      before do
+        expect(Gitlab).to receive(:com?).and_return(true)
+      end
+
+      it { expect(event_data(user, :create)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username, :email_opted_in, :email_opted_in_ip, :email_opted_in_source, :email_opted_in_at) }
+      it { expect(event_data(user, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username, :email_opted_in, :email_opted_in_ip, :email_opted_in_source, :email_opted_in_at) }
+    end
+
+    context 'for non-GitLab.com' do
+      before do
+        expect(Gitlab).to receive(:com?).and_return(false)
+      end
+
+      it { expect(event_data(user, :create)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username) }
+      it { expect(event_data(user, :destroy)).to include(:event_name, :name, :created_at, :updated_at, :email, :user_id, :username) }
+    end
+  end
+
+  def event_data(*args)
+    SystemHooksService.new.send :build_event_data, *args
+  end
+end