Skip to content

Commit e1cdee9

Browse files
committed
Create EmailConfirmationHandler concern & refactor
This change addresses the duplicate code shared between `app/controllers/sessions_controller.rb` and `app/controllers/users/omniauth_callbacks_controller.rb`. It does so by creating `app/models/concerns/email_confirmation_handler.rb` which allows us to better adhere to DRY principles. - Additionally, the `user.confirmed? || user.confirmation_token.present?` check has been moved from the User model to the concern. It made sense to have this check as a User method. However, the method itself is simply an or check of two other User methods, and only the `EmailConfirmationHandler` module is currently needing it.
1 parent 6f0924e commit e1cdee9

File tree

4 files changed

+38
-30
lines changed

4 files changed

+38
-30
lines changed

app/controllers/sessions_controller.rb

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
# Controller that handles user login and logout
44
class SessionsController < Devise::SessionsController
5+
include EmailConfirmationHandler
6+
57
def new
68
redirect_to(root_path)
79
end
@@ -13,10 +15,8 @@ def create
1315
existing_user = User.find_by(email: params[:user][:email])
1416
if existing_user.present?
1517

16-
unless existing_user.confirmed_or_has_confirmation_token?
17-
handle_missing_confirmation_instructions(existing_user)
18-
return
19-
end
18+
# (see app/models/concerns/email_confirmation_handler.rb)
19+
return if confirmation_instructions_missing_and_handled?(existing_user)
2020

2121
# Until ORCID login is supported
2222
@ui = create_shibboleth_identifier(existing_user) unless session['devise.shibboleth_data'].nil?
@@ -53,11 +53,3 @@ def create_shibboleth_identifier(user)
5353
}
5454
Identifier.new(args)
5555
end
56-
57-
def handle_missing_confirmation_instructions(user)
58-
# Generate a confirmation_token and email confirmation instructions to the user
59-
user.send_confirmation_instructions
60-
# Notify the user they are unconfirmed but confirmation instructions have been sent
61-
flash[:notice] = I18n.t('devise.registrations.signed_up_but_unconfirmed')
62-
redirect_to root_path
63-
end

app/controllers/users/omniauth_callbacks_controller.rb

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
module Users
44
# Controller that handles callbacks from OmniAuth integrations (e.g. Shibboleth and ORCID)
55
class OmniauthCallbacksController < Devise::OmniauthCallbacksController
6+
include EmailConfirmationHandler
67
##
78
# Dynamically build a handler for each omniauth provider
89
# -------------------------------------------------------------
@@ -40,10 +41,10 @@ def handle_omniauth(scheme)
4041
# Otherwise sign them in
4142
elsif scheme.name == 'shibboleth'
4243
# Until ORCID becomes supported as a login method
43-
unless existing_user.confirmed_or_has_confirmation_token?
44-
handle_missing_confirmation_instructions(existing_user)
45-
return
46-
end
44+
45+
# (see app/models/concerns/email_confirmation_handler.rb)
46+
return if confirmation_instructions_missing_and_handled?(user)
47+
4748
set_flash_message(:notice, :success, kind: scheme.description) if is_navigational_format?
4849
sign_in_and_redirect user, event: :authentication
4950
else
@@ -87,14 +88,4 @@ def failure
8788
redirect_to root_path
8889
end
8990
end
90-
91-
private
92-
93-
def handle_missing_confirmation_instructions(user)
94-
# Generate a confirmation_token and email confirmation instructions to the user
95-
user.send_confirmation_instructions
96-
# Notify the user they are unconfirmed but confirmation instructions have been sent
97-
flash[:notice] = I18n.t('devise.registrations.signed_up_but_unconfirmed')
98-
redirect_to root_path
99-
end
10091
end
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# frozen_string_literal: true
2+
3+
# Some users in our db are both unconfirmed AND have no outstanding confirmation_token
4+
# This is true for those users due to the following:
5+
# - We haven't always used Devise's :confirmable module (it generates a confirmation_token when a user is created)
6+
# - We have set `confirmed_at` and `confirmation_token` to nil via Rake tasks (lib/tasks/email_confirmation.rake)
7+
# This concern is meant to streamline the confirmation process for those users
8+
module EmailConfirmationHandler
9+
extend ActiveSupport::Concern
10+
11+
def confirmation_instructions_missing_and_handled?(user)
12+
# A user's "confirmation instructions are missing" if they're both unconfirmed and have no confirmation_token
13+
return false if user_confirmed_or_has_confirmation_token?(user)
14+
15+
handle_missing_confirmation_instructions(user)
16+
true
17+
end
18+
19+
private
20+
21+
def user_confirmed_or_has_confirmation_token?(user)
22+
user.confirmed? || user.confirmation_token.present?
23+
end
24+
25+
def handle_missing_confirmation_instructions(user)
26+
user.send_confirmation_instructions
27+
redirect_to root_path, notice: I18n.t('devise.registrations.signed_up_but_unconfirmed')
28+
end
29+
end

app/models/user.rb

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,10 +382,6 @@ def deliver_invitation(options = {})
382382
)
383383
end
384384

385-
def confirmed_or_has_confirmation_token?
386-
confirmed? || confirmation_token.present?
387-
end
388-
389385
# Case insensitive search over User model
390386
#
391387
# field - The name of the field being queried

0 commit comments

Comments
 (0)