Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f3ce7c8
create view test to verify email and phone number fields show up
xihai01 Jul 10, 2022
83d1947
add phone number field to forgot your password page
xihai01 Jul 10, 2022
c7008cc
initial branch commit
xihai01 Jul 10, 2022
a2d8074
create system tests to verify fields can handle correct inputs
xihai01 Jul 10, 2022
5b28758
set up route to map to custom password controller implemented from de…
xihai01 Jul 13, 2022
f8dbab0
implement form error handling in custom password controller
xihai01 Jul 13, 2022
7c46dc1
add ability to send emails with password reset link in custom passwor…
xihai01 Jul 13, 2022
c48b094
create request specs to verify short io and twilio services hit and a…
xihai01 Jul 14, 2022
a6fa24a
extend password controller create action to send pass reset email and…
xihai01 Jul 14, 2022
ad6bf41
add text in views to notify users they only need to enter an email or…
xihai01 Jul 14, 2022
5ce13d0
refactor controller to allow the option of handling one or the other …
xihai01 Jul 14, 2022
ee24de3
handle edge cases such as no fields entered and only phone number ent…
xihai01 Jul 15, 2022
cfa8434
fix bug with reset pass link
xihai01 Jul 16, 2022
f2db6b9
clean up and lint files
xihai01 Jul 16, 2022
be2b80d
move spec to views so it is semantically consistent
xihai01 Jul 16, 2022
76d0327
add system spec to check for empty form submission
xihai01 Jul 16, 2022
09149e3
refactor passwords controller create action to reduce cognitive compl…
xihai01 Jul 16, 2022
f5b9b6e
lint files
xihai01 Jul 16, 2022
a90ec31
reduce cognitive complexity
xihai01 Jul 16, 2022
90c0468
migrate request spec to controller spec
xihai01 Jul 16, 2022
ee1954e
lint files
xihai01 Jul 16, 2022
b608904
refactor password controller for cognitive complexity again
xihai01 Jul 16, 2022
a363799
refactor req params fetching to reduce brain complexity
xihai01 Jul 17, 2022
45bcefe
99% sure brain complexity is reduced in passwords controller
xihai01 Jul 17, 2022
3fc478d
recreate db so schema is up to date with latest changes - should fix …
xihai01 Jul 19, 2022
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
71 changes: 71 additions & 0 deletions app/controllers/users/passwords_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
class Users::PasswordsController < Devise::PasswordsController
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

eek

include ApplicationHelper
include PhoneNumberHelper
include SmsBodyHelper

def create
email, phone_number = [params[resource_name][:email], params[resource_name][:phone_number]]
@resource = email.blank? ? User.find_by(phone_number: phone_number) : User.find_by(email: email)

# re-render and display any errors
params_is_valid, error_resource = password_params_is_valid(resource, email, phone_number)
if !params_is_valid
respond_with(error_resource)
return
end

# generate a reset token and
# call devise mailer
reset_token = send_email_reset(email)
# for case where user enters ONLY a phone number, generate a new reset token to use;
# otherwise, use the same reset token as sent by devise mailer
send_sms_reset(@resource, phone_number, reset_token)
redirect_to after_sending_reset_password_instructions_path_for(resource_name), notice: "You will receive an email or SMS with instructions on how to reset your password in a few minutes."
end

private

def send_email_reset(email)
reset_token = nil
if !email.blank?
reset_token = @resource.send_reset_password_instructions
end
reset_token
end

def send_sms_reset(resource, phone_number, reset_token)
if !phone_number.blank?
reset_token ||= resource.generate_password_reset_token
short_io_service = ShortUrlService.new
short_io_service.create_short_url(request.base_url + "/users/password/edit?reset_password_token=#{reset_token}")
twilio_service = TwilioService.new(resource.casa_org.twilio_api_key_sid, resource.casa_org.twilio_api_key_secret, resource.casa_org.twilio_account_sid)
sms_params = {
From: resource.casa_org.twilio_phone_number,
Body: password_reset_msg(resource.display_name, short_io_service.short_url),
To: phone_number
}
twilio_service.send_sms(sms_params)
end
end

def password_params_is_valid(resource, email, phone_number)
if email.blank? && phone_number.blank?
resource.errors.add(:base, "Please enter at least one field.")
return [false, resource]
end

phone_number_is_valid, error_message = valid_phone_number(phone_number)
if !phone_number_is_valid
resource.errors.add(:phone_number, error_message)
return [false, resource]
end

if resource.email != email || resource.phone_number != phone_number
# A new, empty resource is returned (see application helper)
# so to check for nil, we need to check its email/phone fields
resource.errors.add(:base, "User does not exist.")
return [false, resource]
end
[true, nil]
end
end
12 changes: 12 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,16 @@ def flash_class(level)
def og_tag(type, options = {})
tag.meta(property: "og:#{type}", **options)
end

def resource_name
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

for devise ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes, for devise. The devise view calls some internal methods like resource and resource_name. So we need to map these to use them in our controllers and specs

:user
end

def resource
@resource ||= User.new
end

def devise_mapping
@devise_mapping ||= Devise.mappings[:user]
end
end
4 changes: 4 additions & 0 deletions app/helpers/sms_body_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,8 @@ def no_contact_made_msg(contact_type, short_link)
def case_contact_flagged_msg(display_name, short_link)
"#{display_name} has flagged a Case Contact that needs follow up. Click to see more: #{short_link}"
end

def password_reset_msg(display_name, short_link)
"Hi #{display_name}, click here to reset your password: #{short_link}"
end
end
4 changes: 2 additions & 2 deletions app/models/casa_case_contact_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ class CasaCaseContactType < ApplicationRecord
# id :bigint not null, primary key
# created_at :datetime not null
# updated_at :datetime not null
# casa_case_id :bigint not null
# contact_type_id :bigint not null
# casa_case_id :integer not null
# contact_type_id :integer not null
#
# Indexes
#
Expand Down
4 changes: 2 additions & 2 deletions app/models/case_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ def casa_case_and_volunteer_must_belong_to_same_casa_org
# hide_old_contacts :boolean default(FALSE)
# created_at :datetime not null
# updated_at :datetime not null
# casa_case_id :bigint not null
# volunteer_id :bigint not null
# casa_case_id :integer not null
# volunteer_id :integer not null
#
# Indexes
#
Expand Down
4 changes: 2 additions & 2 deletions app/models/case_contact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ def self.options_for_sorted_by
# want_driving_reimbursement :boolean default(FALSE)
# created_at :datetime not null
# updated_at :datetime not null
# casa_case_id :bigint not null
# creator_id :bigint not null
# casa_case_id :integer not null
# creator_id :integer not null
#
# Indexes
#
Expand Down
4 changes: 2 additions & 2 deletions app/models/case_contact_contact_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ class CaseContactContactType < ApplicationRecord
# id :bigint not null, primary key
# created_at :datetime not null
# updated_at :datetime not null
# case_contact_id :bigint not null
# contact_type_id :bigint not null
# case_contact_id :integer not null
# contact_type_id :integer not null
#
# Indexes
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/contact_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class ContactType < ApplicationRecord
# name :string not null
# created_at :datetime not null
# updated_at :datetime not null
# contact_type_group_id :bigint not null
# contact_type_group_id :integer not null
#
# Indexes
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/contact_type_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def generate_for_org!(casa_org)
# name :string not null
# created_at :datetime not null
# updated_at :datetime not null
# casa_org_id :bigint not null
# casa_org_id :integer not null
#
# Indexes
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/court_date.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def case_court_orders_context_hash
# date :datetime not null
# created_at :datetime not null
# updated_at :datetime not null
# casa_case_id :bigint not null
# casa_case_id :integer not null
# hearing_type_id :bigint
# judge_id :bigint
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/emancipation_option.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class EmancipationOption < ApplicationRecord
# name :string not null
# created_at :datetime not null
# updated_at :datetime not null
# emancipation_category_id :bigint not null
# emancipation_category_id :integer not null
#
# Indexes
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/hearing_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def generate_for_org!(casa_org)
# active :boolean default(TRUE), not null
# checklist_updated_date :string default("None"), not null
# name :string not null
# casa_org_id :bigint not null
# casa_org_id :integer not null
#
# Indexes
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/judge.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class Judge < ApplicationRecord
# name :string
# created_at :datetime not null
# updated_at :datetime not null
# casa_org_id :bigint not null
# casa_org_id :integer not null
#
# Indexes
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/supervisor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def recently_unassigned_volunteers
# created_at :datetime not null
# updated_at :datetime not null
# casa_org_id :bigint not null
# invited_by_id :bigint
# invited_by_id :integer
#
# Indexes
#
Expand Down
4 changes: 2 additions & 2 deletions app/models/supervisor_volunteer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ def ensure_supervisor_and_volunteer_belong_to_same_casa_org
# is_active :boolean default(TRUE)
# created_at :datetime not null
# updated_at :datetime not null
# supervisor_id :bigint not null
# volunteer_id :bigint not null
# supervisor_id :integer not null
# volunteer_id :integer not null
#
# Indexes
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def serving_transition_aged_youth?
# created_at :datetime not null
# updated_at :datetime not null
# casa_org_id :bigint not null
# invited_by_id :bigint
# invited_by_id :integer
#
# Indexes
#
Expand Down
2 changes: 1 addition & 1 deletion app/models/volunteer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def cases_where_contact_made_in_days(num_days = CONTACT_MADE_IN_DAYS_NUM)
# created_at :datetime not null
# updated_at :datetime not null
# casa_org_id :bigint not null
# invited_by_id :bigint
# invited_by_id :integer
#
# Indexes
#
Expand Down
9 changes: 8 additions & 1 deletion app/views/devise/passwords/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,16 @@
<%= form_for(resource, as: resource_name, url: password_path(resource_name), html: {method: :post}) do |f| %>
<%= render "/shared/error_messages", resource: resource %>

<h4>Please enter email or phone number to recieve reset instructions.</h4>

<div class="field form-group">
<%= f.label :email %><br>
<%= f.email_field :email, autofocus: true, autocomplete: "email", class: "form-control", required: true %>
<%= f.email_field :email, autofocus: true, autocomplete: "email", class: "form-control" %>
</div>

<div class="field form-group">
<%= f.label :phone_number %><br>
<%= f.text_field :phone_number, autofocus: true, autocomplete: "phone number", class: "form-control" %>
</div>

<div class="actions">
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Rails.application.routes.draw do
devise_for :all_casa_admins, path: "all_casa_admins", controllers: {sessions: "all_casa_admins/sessions"}
devise_for :users, controllers: {sessions: "users/sessions"}
devise_for :users, controllers: {sessions: "users/sessions", passwords: "users/passwords"}

concern :with_datatable do
post "datatable", on: :collection
Expand Down
32 changes: 32 additions & 0 deletions spec/controllers/users/passwords_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
require "rails_helper"
require "support/stubbed_requests/webmock_helper"

RSpec.describe Users::PasswordsController, type: :controller do
describe "create" do
before do
stubbed_sites = ["api.twilio.com", "api.short.io"]
web_mock = WebMockHelper.new(stubbed_sites)
web_mock.stub_network_connection
end

it "sends a password reset SMS to existing user" do
org = create(:casa_org)
user = create(:user, phone_number: "+12222222222", casa_org: org)

@short_io_stub = WebMockHelper.short_io_stub_sms
@twilio_stub = WebMockHelper.twilio_password_reset_stub(user)

params = {
user: {
email: user.email,
phone_number: user.phone_number
}
}

post :create, params: params
expect(@short_io_stub).to have_been_requested.times(1)
expect(@twilio_stub).to have_been_requested.times(1)
expect(response).to have_http_status(:redirect)
end
end
end
12 changes: 12 additions & 0 deletions spec/support/stubbed_requests/twilio_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,16 @@ def twilio_no_contact_made_stub(resousne = "")
)
.to_return(body: "{\"error_code\":null, \"status\":\"sent\", \"body\":\"It's been two weeks since you've tried reaching 'test'. Try again! https://42ni.short.gy/jzTwdF\"}")
end

def twilio_password_reset_stub(resource)
WebMock.stub_request(:post, "https://api.twilio.com/2010-04-01/Accounts/articuno34/Messages.json")
.with(
body: {From: "+15555555555", Body: "Hi #{resource.display_name}, click here to reset your password: https://42ni.short.gy/jzTwdF", To: "+12222222222"},
headers: {
"Content-Type" => "application/x-www-form-urlencoded",
"Authorization" => "Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ=="
}
)
.to_return(body: "{\"error_code\":null, \"status\":\"sent\", \"body\":\"Execute Order 66 - https://42ni.short.gy/jzTwdF\"}")
end
end
40 changes: 40 additions & 0 deletions spec/system/devise/passwords/new_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
require "rails_helper"

RSpec.describe "users/passwords/new", type: :system do
before do
visit root_path
click_on "Forgot your password?"
end

it "displays error messages for non-existent user" do
fill_in "Email", with: "tangerine@forward.com"
fill_in "Phone number", with: "+16578900012"

click_on "Send me reset password instructions"
expect(page).to have_content "1 error prohibited this User from being saved:"
expect(page).to have_text("User does not exist.")
end

it "displays phone number error messages for incorrect formatting" do
create(:user, email: "glados@aperture.labs")
fill_in "Email", with: "glados@aperture.labs"
fill_in "Phone number", with: "2134567eee"

click_on "Send me reset password instructions"
expect(page).to have_content "1 error prohibited this User from being saved:"
expect(page).to have_text("Phone number must be 12 digits including country code (+1)")
end

it "displays error if user tries to submit empty form" do
click_on "Send me reset password instructions"
expect(page).to have_text("Please enter at least one field.")
end

it "redirects to sign up page for email" do
create(:user, email: "glados@aperture.labs")
fill_in "Email", with: "glados@aperture.labs"

click_on "Send me reset password instructions"
expect(page).to have_content "You will receive an email or SMS with instructions on how to reset your password in a few minutes."
end
end
21 changes: 21 additions & 0 deletions spec/views/devise/passwords/new.html.erb_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
require "rails_helper"

RSpec.describe "users/password/new", type: :view do
it "displays title" do
render template: "devise/passwords/new"
expect(rendered).to have_text("Forgot your password?")
end

it "displays text above form fields" do
render template: "devise/passwords/new"
expect(rendered).to have_text("Please enter email or phone number to recieve reset instructions.")
end

it "displays contact fields for user to reset password" do
render template: "devise/passwords/new"
expect(rendered).to have_text("Email")
expect(rendered).to have_field("user_email")
expect(rendered).to have_text("Phone number")
expect(rendered).to have_field("user_phone_number")
end
end