From c5526a2d9a946d99d7b4a72fc488fe6e0a9ad60b Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Thu, 28 Apr 2016 17:09:15 -0300 Subject: [PATCH] Change skip_user_confirmation_email to send_user_confirmation_email --- CHANGELOG | 5 ++++- .../admin/application_settings_controller.rb | 2 +- app/models/user.rb | 3 ++- app/views/admin/application_settings/_form.html.haml | 4 ++-- ...end_confirmation_email_to_application_settings.rb | 12 ++++++++++++ ...kip_confirmation_email_to_application_settings.rb | 8 -------- spec/controllers/registrations_controller_spec.rb | 12 ++++++------ spec/features/signup_spec.rb | 2 ++ spec/models/user_spec.rb | 1 + 9 files changed, 30 insertions(+), 19 deletions(-) create mode 100644 db/migrate/20160421141709_add_send_confirmation_email_to_application_settings.rb delete mode 100644 db/migrate/20160421141709_add_skip_confirmation_email_to_application_settings.rb diff --git a/CHANGELOG b/CHANGELOG index 7a86263c0a9..5841da3ac99 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -28,6 +28,8 @@ v 8.8.0 (unreleased) - Display informative message when new milestone is created - Sanitize milestones and labels titles - Support multi-line tag messages. !3833 (Calin Seciu) + - Toggle sign-up confirmation emails in application settings + - Replace Devise Async with Devise ActiveJob integration. !3902 (Connor Shea) - Allow "NEWS" and "CHANGES" as alternative names for CHANGELOG. !3768 (Connor Shea) - Added button to toggle whitespaces changes on diff view - Backport GitHub Enterprise import support from EE @@ -93,6 +95,8 @@ v 8.7.1 v 8.7.0 - Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented - Fix vulnerability that made it possible to gain access to private labels and milestones + +v 8.7.0 (unreleased) - The number of InfluxDB points stored per UDP packet can now be configured - Fix error when cross-project label reference used with non-existent project - Transactions for /internal/allowed now have an "action" tag set @@ -189,7 +193,6 @@ v 8.7.0 - Add Slack notifications when Wiki is edited (Sebastian Klier) - Diffs load at the correct point when linking from from number - Selected diff rows highlight - - Toggle sign-up confirmation emails in application settings - Fix emoji categories in the emoji picker - API: Properly display annotated tags for GET /projects/:id/repository/tags (Robert Schilling) - Add encrypted credentials for imported projects and migrate old ones diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 956d145f029..ff7a5cad2fb 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -106,7 +106,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :email_author_in_body, :repository_checks_enabled, :metrics_packet_size, - :skip_user_confirmation_email, + :send_user_confirmation_email, restricted_visibility_levels: [], import_sources: [], disabled_oauth_sign_in_sources: [] diff --git a/app/models/user.rb b/app/models/user.rb index 470734f5c2f..bcadbd3e2fb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -309,7 +309,8 @@ class User < ActiveRecord::Base end def check_confirmation_email - skip_confirmation! if current_application_settings.skip_user_confirmation_email + byebug + skip_confirmation! unless current_application_settings.send_user_confirmation_email end def recently_sent_password_reset? diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index 6d6d87cdd5a..289dda9a436 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -107,8 +107,8 @@ .col-sm-offset-2.col-sm-10 .checkbox = f.label :skip_confirmation_email do - = f.check_box :skip_user_confirmation_email - Skip sign-up email confirmation + = f.check_box :send_user_confirmation_email + Send sign-up email confirmation .form-group .col-sm-offset-2.col-sm-10 .checkbox diff --git a/db/migrate/20160421141709_add_send_confirmation_email_to_application_settings.rb b/db/migrate/20160421141709_add_send_confirmation_email_to_application_settings.rb new file mode 100644 index 00000000000..0fef2a831e7 --- /dev/null +++ b/db/migrate/20160421141709_add_send_confirmation_email_to_application_settings.rb @@ -0,0 +1,12 @@ +class AddSendConfirmationEmailToApplicationSettings < ActiveRecord::Migration + def up + add_column :application_settings, :send_user_confirmation_email, :boolean, default: false + + #Sets confirmation email to true by default on existing installations. + ApplicationSetting.update_all(send_user_confirmation_email: true) + end + + def down + remove_column :application_settings, :send_user_confirmation_email + end +end diff --git a/db/migrate/20160421141709_add_skip_confirmation_email_to_application_settings.rb b/db/migrate/20160421141709_add_skip_confirmation_email_to_application_settings.rb deleted file mode 100644 index 953f1cea896..00000000000 --- a/db/migrate/20160421141709_add_skip_confirmation_email_to_application_settings.rb +++ /dev/null @@ -1,8 +0,0 @@ -class AddSkipConfirmationEmailToApplicationSettings < ActiveRecord::Migration - def change - #Skip confirmation emails just for new installations - default_value = User.count > 0 ? false : true - - add_column :application_settings, :skip_user_confirmation_email, :boolean, default: default_value - end -end diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index b4ab767f73e..29f1847d9a1 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -8,10 +8,10 @@ describe RegistrationsController do end end - let(:user_params) { { "user"=> {"name"=>"new_user", "username"=>"new_username", "email"=>"new@user.com", "password"=>"Any_password"} } } + let(:user_params) { { user: { name: "new_user", username: "new_username", email: "new@user.com", password: "Any_password" } } } - context 'when skipping email confirmation' do - before { allow(current_application_settings).to receive(:skip_user_confirmation_email).and_return(true) } + context 'when sending email confirmation' do + before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(false) } it 'logs user in directly' do post(:create, user_params) @@ -20,12 +20,12 @@ describe RegistrationsController do end end - context 'when not skipping email confirmation' do - before { allow(current_application_settings).to receive(:skip_user_confirmation_email).and_return(false) } + context 'when not sending email confirmation' do + before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(true) } it 'does not authenticate user and sends confirmation email' do post(:create, user_params) - expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params["user"]["email"]) + expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email]) expect(subject.current_user).to be_nil end end diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb index 58aabd913eb..c7840f26d8f 100644 --- a/spec/features/signup_spec.rb +++ b/spec/features/signup_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' feature 'Signup', feature: true do describe 'signup with no errors' do + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) } + it 'creates the user account and sends a confirmation email' do user = build(:user) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 10e7e693571..9581990666b 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -141,6 +141,7 @@ describe User, models: true do end describe '#confirm' do + before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(true) } let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'test@gitlab.com') } it 'returns unconfirmed' do -- GitLab