Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions app/controllers/concerns/unconfirmed_user_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ def email_confirmation_token_validator

def process_valid_confirmation_token
@confirmation_token = params[:confirmation_token]
@forbidden_passwords = @user.email_addresses.flat_map do |email_address|
ForbiddenPasswords.new(email_address.email).call
end
flash.now[:success] = t('devise.confirmations.confirmed_but_must_set_password')
session[:user_confirmation_token] = @confirmation_token
end

Expand Down
19 changes: 7 additions & 12 deletions app/controllers/sign_up/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ class PasswordsController < ApplicationController

def new
password_form # Memoize the password form to use in the view
process_successful_confirmation
process_valid_confirmation_token
flash.now[:success] = t('devise.confirmations.confirmed_but_must_set_password')
@forbidden_passwords = forbidden_passwords
end

def create
Expand All @@ -28,17 +30,10 @@ def create

private

def process_successful_confirmation
process_valid_confirmation_token
render_page
end

def render_page
render(
:new,
locals: { confirmation_token: @confirmation_token },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT we weren't ever using this locals assignment, since the view references @confirmation_token directly. Locals are intended to be referenced without the @.

formats: :html,
)
def forbidden_passwords
@user.email_addresses.flat_map do |email_address|
ForbiddenPasswords.new(email_address.email).call
end
end

def track_analytics(result)
Expand Down
59 changes: 41 additions & 18 deletions spec/controllers/sign_up/passwords_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,47 @@
RSpec.describe SignUp::PasswordsController do
let(:token) { 'new token' }

describe '#new' do
let!(:user) { create(:user, :unconfirmed, confirmation_token: token) }
subject(:response) { get :new, params: { confirmation_token: token } }

it 'flashes a message informing the user that they need to set a password' do
response

expect(flash.now[:success]).to eq(t('devise.confirmations.confirmed_but_must_set_password'))
end

it 'processes valid token' do
expect(controller).to receive(:process_valid_confirmation_token)

response
end

it 'assigns variables expected to be available in the view' do
response

expect(assigns(:password_form)).to be_instance_of(PasswordForm)
expect(assigns(:email_address)).to be_instance_of(EmailAddress)
expect(assigns(:forbidden_passwords)).to be_present.and all be_kind_of(String)
expect(assigns(:confirmation_token)).to be_kind_of(String)
end

context 'with invalid confirmation_token' do
let!(:user) do
create(
:user,
:unconfirmed,
confirmation_token: token,
confirmation_sent_at: (IdentityConfig.store.add_email_link_valid_for_hours + 1).hours.ago,
)
end

it 'redirects to sign up page' do
expect(response).to redirect_to(sign_up_register_url)
end
end
end

describe '#create' do
subject(:response) { post :create, params: params }
let(:params) do
Expand Down Expand Up @@ -143,22 +184,4 @@
end
end
end

describe '#new' do
render_views

it 'rejects when confirmation_token is invalid' do
invalid_confirmation_sent_at =
Time.zone.now - (IdentityConfig.store.add_email_link_valid_for_hours.hours.in_seconds + 1)
create(
:user,
:unconfirmed,
confirmation_token: token,
confirmation_sent_at: invalid_confirmation_sent_at,
)

get :new, params: { confirmation_token: token }
expect(response).to redirect_to(sign_up_register_url)
end
end
end