Commit 19af0ffc authored by Ash McKenzie's avatar Ash McKenzie

Merge branch '201893-resolve-soft-email-confirmation-flow' into 'master'

Remove the soft_email_confirmation feature flag

See merge request gitlab-org/gitlab!24371
parents 5c2c6c72 9af41e99
...@@ -10,7 +10,7 @@ module ConfirmEmailWarning ...@@ -10,7 +10,7 @@ module ConfirmEmailWarning
protected protected
def show_confirm_warning? def show_confirm_warning?
html_request? && request.get? && Feature.enabled?(:soft_email_confirmation) html_request? && request.get?
end end
def set_confirm_warning def set_confirm_warning
......
...@@ -11,7 +11,7 @@ class ConfirmationsController < Devise::ConfirmationsController ...@@ -11,7 +11,7 @@ class ConfirmationsController < Devise::ConfirmationsController
protected protected
def after_resending_confirmation_instructions_path_for(resource) def after_resending_confirmation_instructions_path_for(resource)
Feature.enabled?(:soft_email_confirmation) ? stored_location_for(resource) || dashboard_projects_path : users_almost_there_path stored_location_for(resource) || dashboard_projects_path
end end
def after_confirmation_path_for(resource_name, resource) def after_confirmation_path_for(resource_name, resource)
......
...@@ -53,7 +53,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -53,7 +53,7 @@ class RegistrationsController < Devise::RegistrationsController
def welcome def welcome
return redirect_to new_user_registration_path unless current_user return redirect_to new_user_registration_path unless current_user
return redirect_to stored_location_or_dashboard_or_almost_there_path(current_user) if current_user.role.present? && !current_user.setup_for_company.nil? return redirect_to stored_location_or_dashboard(current_user) if current_user.role.present? && !current_user.setup_for_company.nil?
current_user.name = nil if current_user.name == current_user.username current_user.name = nil if current_user.name == current_user.username
end end
...@@ -65,7 +65,7 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -65,7 +65,7 @@ class RegistrationsController < Devise::RegistrationsController
if result[:status] == :success if result[:status] == :success
track_experiment_event(:signup_flow, 'end') # We want this event to be tracked when the user is _in_ the experimental group track_experiment_event(:signup_flow, 'end') # We want this event to be tracked when the user is _in_ the experimental group
set_flash_message! :notice, :signed_up set_flash_message! :notice, :signed_up
redirect_to stored_location_or_dashboard_or_almost_there_path(current_user) redirect_to stored_location_or_dashboard(current_user)
else else
render :welcome render :welcome
end end
...@@ -112,12 +112,12 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -112,12 +112,12 @@ class RegistrationsController < Devise::RegistrationsController
return users_sign_up_welcome_path if experiment_enabled?(:signup_flow) return users_sign_up_welcome_path if experiment_enabled?(:signup_flow)
stored_location_or_dashboard_or_almost_there_path(user) stored_location_or_dashboard(user)
end end
def after_inactive_sign_up_path_for(resource) def after_inactive_sign_up_path_for(resource)
Gitlab::AppLogger.info(user_created_message) Gitlab::AppLogger.info(user_created_message)
Feature.enabled?(:soft_email_confirmation) ? dashboard_projects_path : users_almost_there_path dashboard_projects_path
end end
private private
...@@ -179,18 +179,10 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -179,18 +179,10 @@ class RegistrationsController < Devise::RegistrationsController
Gitlab::Utils.to_boolean(params[:terms_opt_in]) Gitlab::Utils.to_boolean(params[:terms_opt_in])
end end
def confirmed_or_unconfirmed_access_allowed(user)
user.confirmed? || Feature.enabled?(:soft_email_confirmation) || experiment_enabled?(:signup_flow)
end
def stored_location_or_dashboard(user) def stored_location_or_dashboard(user)
stored_location_for(user) || dashboard_projects_path stored_location_for(user) || dashboard_projects_path
end end
def stored_location_or_dashboard_or_almost_there_path(user)
confirmed_or_unconfirmed_access_allowed(user) ? stored_location_or_dashboard(user) : users_almost_there_path
end
# Part of an experiment to build a new sign up flow. Will be resolved # Part of an experiment to build a new sign up flow. Will be resolved
# with https://gitlab.com/gitlab-org/growth/engineering/issues/64 # with https://gitlab.com/gitlab-org/growth/engineering/issues/64
def choose_layout def choose_layout
......
...@@ -1638,13 +1638,6 @@ class User < ApplicationRecord ...@@ -1638,13 +1638,6 @@ class User < ApplicationRecord
super super
end end
# override from Devise::Confirmable
def confirmation_period_valid?
return false if Feature.disabled?(:soft_email_confirmation)
super
end
private private
def default_private_profile_to_false def default_private_profile_to_false
......
---
title: Allow a grace period for new users to confirm their email
merge_request: 24371
author:
type: added
...@@ -3,10 +3,6 @@ ...@@ -3,10 +3,6 @@
require 'spec_helper' require 'spec_helper'
describe ConfirmEmailWarning do describe ConfirmEmailWarning do
before do
stub_feature_flags(soft_email_confirmation: true)
end
controller(ApplicationController) do controller(ApplicationController) do
# `described_class` is not available in this context # `described_class` is not available in this context
include ConfirmEmailWarning include ConfirmEmailWarning
......
...@@ -77,25 +77,6 @@ describe RegistrationsController do ...@@ -77,25 +77,6 @@ describe RegistrationsController do
context 'when send_user_confirmation_email is true' do context 'when send_user_confirmation_email is true' do
before do before do
stub_application_setting(send_user_confirmation_email: true) stub_application_setting(send_user_confirmation_email: true)
end
context 'when soft email confirmation is not enabled' do
before do
stub_feature_flags(soft_email_confirmation: false)
allow(User).to receive(:allow_unconfirmed_access_for).and_return 0
end
it 'does not authenticate the user and sends a confirmation email' do
post(:create, params: user_params)
expect(ActionMailer::Base.deliveries.last.to.first).to eq(user_params[:user][:email])
expect(subject.current_user).to be_nil
end
end
context 'when soft email confirmation is enabled' do
before do
stub_feature_flags(soft_email_confirmation: true)
allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
end end
...@@ -106,7 +87,6 @@ describe RegistrationsController do ...@@ -106,7 +87,6 @@ describe RegistrationsController do
expect(response).to redirect_to(dashboard_projects_path) expect(response).to redirect_to(dashboard_projects_path)
end end
end end
end
context 'when signup_enabled? is false' do context 'when signup_enabled? is false' do
it 'redirects to sign_in' do it 'redirects to sign_in' do
......
...@@ -153,24 +153,6 @@ describe 'Invites' do ...@@ -153,24 +153,6 @@ describe 'Invites' do
context 'email confirmation enabled' do context 'email confirmation enabled' do
let(:send_email_confirmation) { true } let(:send_email_confirmation) { true }
context 'when soft email confirmation is not enabled' do
before do
# stub_feature_flags(soft_email_confirmation: false)
allow(User).to receive(:allow_unconfirmed_access_for).and_return 0
end
it 'signs up and redirects to root page with all the project/groups invitation automatically accepted' do
fill_in_sign_up_form(new_user)
confirm_email(new_user)
fill_in_sign_in_form(new_user)
expect(current_path).to eq(root_path)
expect(page).to have_content(project.full_name)
visit group_path(group)
expect(page).to have_content(group.full_name)
end
end
context 'when soft email confirmation is enabled' do context 'when soft email confirmation is enabled' do
before do before do
allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
...@@ -198,24 +180,7 @@ describe 'Invites' do ...@@ -198,24 +180,7 @@ describe 'Invites' do
context 'the user sign-up using a different email address' do context 'the user sign-up using a different email address' do
let(:invite_email) { build_stubbed(:user).email } let(:invite_email) { build_stubbed(:user).email }
context 'when soft email confirmation is not enabled' do
before do before do
stub_feature_flags(soft_email_confirmation: false)
allow(User).to receive(:allow_unconfirmed_access_for).and_return 0
end
it 'signs up and redirects to the invitation page' do
fill_in_sign_up_form(new_user)
confirm_email(new_user)
fill_in_sign_in_form(new_user)
expect(current_path).to eq(invite_path(group_invite.raw_invite_token))
end
end
context 'when soft email confirmation is enabled' do
before do
stub_feature_flags(soft_email_confirmation: true)
allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days allow(User).to receive(:allow_unconfirmed_access_for).and_return 2.days
end end
...@@ -227,5 +192,4 @@ describe 'Invites' do ...@@ -227,5 +192,4 @@ describe 'Invites' do
end end
end end
end end
end
end end
...@@ -797,7 +797,6 @@ describe 'Login' do ...@@ -797,7 +797,6 @@ describe 'Login' do
before do before do
stub_application_setting(send_user_confirmation_email: true) stub_application_setting(send_user_confirmation_email: true)
stub_feature_flags(soft_email_confirmation: true)
allow(User).to receive(:allow_unconfirmed_access_for).and_return grace_period allow(User).to receive(:allow_unconfirmed_access_for).and_return grace_period
end end
......
...@@ -129,39 +129,6 @@ shared_examples 'Signup' do ...@@ -129,39 +129,6 @@ shared_examples 'Signup' do
stub_application_setting(send_user_confirmation_email: true) stub_application_setting(send_user_confirmation_email: true)
end end
context 'when soft email confirmation is not enabled' do
before do
stub_feature_flags(soft_email_confirmation: false)
end
it 'creates the user account and sends a confirmation email' do
visit new_user_registration_path
fill_in 'new_user_username', with: new_user.username
fill_in 'new_user_email', with: new_user.email
if Gitlab::Experimentation.enabled?(:signup_flow)
fill_in 'new_user_first_name', with: new_user.first_name
fill_in 'new_user_last_name', with: new_user.last_name
else
fill_in 'new_user_name', with: new_user.name
fill_in 'new_user_email_confirmation', with: new_user.email
end
fill_in 'new_user_password', with: new_user.password
expect { click_button 'Register' }.to change { User.count }.by(1)
expect(current_path).to eq users_almost_there_path
expect(page).to have_content('Please check your email to confirm your account')
end
end
context 'when soft email confirmation is enabled' do
before do
stub_feature_flags(soft_email_confirmation: true)
end
it 'creates the user account and sends a confirmation email' do it 'creates the user account and sends a confirmation email' do
visit new_user_registration_path visit new_user_registration_path
...@@ -188,7 +155,6 @@ shared_examples 'Signup' do ...@@ -188,7 +155,6 @@ shared_examples 'Signup' do
end end
end end
end end
end
context "when sigining up with different cased emails" do context "when sigining up with different cased emails" do
it "creates the user successfully" do it "creates the user successfully" do
......
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